Don't clarify precedence when operators is the same

Author: captbaritone

src/parens.ts

@@ -1,13 +1,12 @@
 import * as vscode from "vscode";
-import { findParens } from "./parse";
+import { findParens, ParenLocations } from "./parse";
 import { debounce, repeat } from "./utils";
 import {
   DEBOUNCE_CONFIG,
   USE_FLOW_CONFIG,
   PAREN_COLOR_ID,
   SUPPORTED_LANGUAGE_IDS,
 } from "./constants";
-import { ParserPlugin } from "@babel/parser";
 
 export function activateParens() {
   const subscriptions = [];
@@ -27,12 +26,18 @@ export function activateParens() {
 
   subscriptions.push(decorationType);
 
+  const outputChannel = vscode.window.createOutputChannel(
+    "Implicit Parentheses"
+  );
+  subscriptions.push(outputChannel);
+
   for (const editor of vscode.window.visibleTextEditors) {
-    updateDecorations(editor, decorationType);
+    updateDecorations(editor, decorationType, outputChannel);
   }
 
   const triggerUpdateDecorations = debounce(
-    (editor: vscode.TextEditor) => updateDecorations(editor, decorationType),
+    (editor: vscode.TextEditor) =>
+      updateDecorations(editor, decorationType, outputChannel),
     vscode.workspace.getConfiguration().get(DEBOUNCE_CONFIG)
   );
   subscriptions.push(
@@ -57,7 +62,14 @@ export function activateParens() {
         activeEditor !== undefined &&
         event.document === activeEditor.document
       ) {
-        triggerUpdateDecorations(activeEditor);
+        const { languageId } = activeEditor.document;
+        if (SUPPORTED_LANGUAGE_IDS.has(languageId)) {
+          triggerUpdateDecorations(activeEditor);
+        } else {
+          outputChannel.appendLine(
+            `The language ${languageId} is not supported by Implicit Parentheses`
+          );
+        }
       }
     },
     null,
@@ -69,24 +81,28 @@ export function activateParens() {
 
 function updateDecorations(
   editor: vscode.TextEditor,
-  decorationType: vscode.TextEditorDecorationType
+  decorationType: vscode.TextEditorDecorationType,
+  outputChannel: vscode.OutputChannel
 ) {
-  const { languageId } = editor.document;
-  if (!SUPPORTED_LANGUAGE_IDS.has(languageId)) {
-    // TODO: Log
-    return;
-  }
+  outputChannel.appendLine("Updating decorations...");
+
   const useFlow: boolean =
     vscode.workspace.getConfiguration().get(USE_FLOW_CONFIG) ?? false;
 
-  const parens = findParens(
-    editor.document.getText(),
-    editor.document.languageId,
-    useFlow
-  );
-  if (parens === null) {
+  let parens: ParenLocations;
+  try {
+    parens = findParens(
+      editor.document.getText(),
+      editor.document.languageId,
+      useFlow
+    );
+  } catch (e) {
+    outputChannel.appendLine(
+      `Encounterd an while parsing ${editor.document.fileName}}: ${String(e)}`
+    );
     return;
   }
+
   const { open, close } = parens;
 
   const decorations: {

src/parse.ts

@@ -3,21 +3,22 @@ import traverse from "@babel/traverse";
 import { File as FileAST, Node } from "@babel/types";
 import { ParserOptions, ParserPlugin } from "@babel/parser";
 
-type ParenLocations = Map<number, number>;
-export type Parens = { open: ParenLocations; close: ParenLocations };
+type Locations = Map<number, number>;
+export type ParenLocations = { open: Locations; close: Locations };
 
 const CONFUSING = new Set([
   "BinaryExpression",
   "LogicalExpression",
   "UnaryExpression",
   "ConditionalExpression",
+  "AssignmentExpression",
 ]);
 
 export function findParens(
   text: string,
   languageId: string,
   useFlow: boolean
-): { open: ParenLocations; close: ParenLocations } | null {
+): ParenLocations {
   const openParens: number[] = [];
   const closeParens: number[] = [];
 
@@ -29,40 +30,50 @@ export function findParens(
     closeParens.push(node.end);
   }
 
-  let ast;
-  try {
-    ast = parser.parse(text, babelOptions({ languageId, useFlow }));
-  } catch (e) {
-    return null;
-  }
+  const ast = parser.parse(text, babelOptions({ languageId, useFlow }));
 
-  const ASSOCIATIVE_BINARY_OPERATORS = new Set(["+", "*", "&", "|"]);
-  const ASSOCIATIVE_LOGICAL_OPERATORS = new Set(["||", "&&", "??"]);
+  const RIGHT_TO_LEFT_ASSOCIATIVE_OPERATORS = new Set([
+    "**",
+    "=",
+    "+=",
+    "-=",
+    "%=",
+    "**=",
+    "*=",
+    "<<=",
+    ">>=",
+    ">>>=",
+    "&=",
+    "^=",
+    "|=",
+    "&&=",
+    "||=",
+    "??=",
+  ]);
 
   traverse(ast, {
     enter(path) {
       if (!CONFUSING.has(path.type) || !CONFUSING.has(path.parent.type)) {
         // People can figure these out probably
         return;
       }
-      if (isUnaryNot(path.node) && isUnaryNot(path.parent)) {
-        // !! is an idiom that people can figure out
-        return;
-      }
-      if (
-        path.node.type === path.parent.type &&
-        // @ts-ignore
-        path.node.operator === path.parent.operator
-      ) {
+      if (path.node.type === path.parent.type) {
         if (
-          (path.node.type === "BinaryExpression" &&
-            ASSOCIATIVE_BINARY_OPERATORS.has(path.node.operator)) ||
-          (path.node.type === "LogicalExpression" &&
-            ASSOCIATIVE_LOGICAL_OPERATORS.has(path.node.operator))
+          // @ts-ignore
+          path.node.operator !== undefined &&
+          // @ts-ignore
+          path.node.operator === path.parent.operator &&
+          // @ts-ignore
+          !RIGHT_TO_LEFT_ASSOCIATIVE_OPERATORS.has(path.node.operator)
         ) {
+          // Here the presedence is obvisouly the same, and people can
+          // infer left-to-right associativity.
           return;
         }
+      } else if (path.parent.type === "AssignmentExpression") {
+        return;
       }
+
       addParens(path.node);
     },
   });
@@ -115,10 +126,6 @@ function groupLineNumbers(numbers: number[]) {
   return counts;
 }
 
-function isUnaryNot(node: Node) {
-  return node.type === "UnaryExpression" && node.operator === "!";
-}
-
 // Stolen from Prettier: https://github.com/prettier/prettier/blob/797e93fc0a3a7f2ba2b510a1a246fc6bdbe89025/src/language-js/parser-babel.js#L18-L41
 function babelOptions({
   languageId,

src/test/unit/parser.test.ts

@@ -1,5 +1,5 @@
 import * as assert from "assert";
-import { findParens, Parens } from "../../parse";
+import { findParens } from "../../parse";
 import { repeat } from "../../utils";
 
 function makeSnapshot(
@@ -34,49 +34,67 @@ suite("Unit Test Suite", () => {
   test("Ignore parenthesized expressions", () => {
     assert.equal(makeSnapshot("1 + (1 * 1)"), "1 + (1 * 1)");
   });
-  test("Ignore associative operators", () => {
+  test("Ignore assignment mixed with other operators", () => {
+    assert.equal(makeSnapshot("1 = 1 * 1"), "1 = 1 * 1");
+  });
+
+  test("Cast to bool idiom", () => {
+    assert.equal(makeSnapshot("!!foo"), "!!foo");
+    assert.equal(makeSnapshot("+!!foo"), "+₍!!foo₎");
+  });
+
+  test("Ignore identical operators if they are left-to-right associative", () => {
     assert.equal(makeSnapshot("1 + 1 + 1"), "1 + 1 + 1");
     assert.equal(makeSnapshot("1 * 1 * 1"), "1 * 1 * 1");
     assert.equal(makeSnapshot("1 && 1 && 1"), "1 && 1 && 1");
     assert.equal(makeSnapshot("1 || 1 || 1"), "1 || 1 || 1");
     assert.equal(makeSnapshot("1 ?? 1 ?? 1"), "1 ?? 1 ?? 1");
     assert.equal(makeSnapshot("1 & 1 & 1"), "1 & 1 & 1");
     assert.equal(makeSnapshot("1 | 1 | 1"), "1 | 1 | 1");
+    assert.equal(makeSnapshot("1 - 1 - 1"), "1 - 1 - 1");
+    assert.equal(makeSnapshot("1 - 1 - 1"), "1 - 1 - 1");
+  });
+
+  test("Don't ignore identical operators if they are right-to-left associative", () => {
+    assert.equal(makeSnapshot("1 ** 1 ** 1"), "1 ** ₍1 ** 1₎");
+    assert.equal(makeSnapshot("a = b = c"), "a = ₍b = c₎");
+    assert.equal(makeSnapshot("a ??= b ??= c"), "a ??= ₍b ??= c₎");
+  });
+
+  test("Nested Ternary", () => {
+    assert.equal(makeSnapshot("a ? b ? c : d : e"), "a ? ₍b ? c : d₎ : e");
+    assert.equal(makeSnapshot("a ? b : c ? d : e"), "a ? b : ₍c ? d : e₎");
+    // assert.equal(makeSnapshot("a ? b : c ? d : e"), "1 ** ₍1 ** 1₎");
   });
 
   // Parser config tests
   test("Flow languageId", () => {
-    assert.notEqual(
-      makeSnapshot("type FooT = {| foo: string |};", "flow", false),
-      null
+    assert.doesNotThrow(() =>
+      makeSnapshot("type FooT = {| foo: string |};", "flow", false)
     );
   });
   test("Flow as default for js", () => {
-    assert.notEqual(
-      makeSnapshot("type FooT = {| foo: string |};", "javascript", true),
-      null
+    assert.doesNotThrow(() =>
+      makeSnapshot("type FooT = {| foo: string |};", "javascript", true)
     );
-    assert.notEqual(
-      makeSnapshot("type FooT = {| foo: string |};", "javascriptreact", true),
-      null
+    assert.doesNotThrow(() =>
+      makeSnapshot("type FooT = {| foo: string |};", "javascriptreact", true)
     );
   });
   test("Typescript cannot parse Flow", () => {
-    assert.equal(
-      makeSnapshot("type FooT = {| foo: string |};", "typescript"),
-      null
+    assert.throws(() =>
+      makeSnapshot("type FooT = {| foo: string |};", "typescript")
     );
   });
   test("TypeScript", () => {
-    assert.notEqual(
-      makeSnapshot("interface FooT { foo: string };", "typescript"),
-      null
+    assert.doesNotThrow(() =>
+      makeSnapshot("interface FooT { foo: string };", "typescript")
     );
   });
 
   test("JSX", () => {
-    assert.equal(makeSnapshot("<Bar />", "typescript"), null);
-    assert.equal(makeSnapshot("<Bar />", "javascript"), null);
+    assert.throws(() => makeSnapshot("<Bar />", "typescript"));
+    assert.throws(() => makeSnapshot("<Bar />", "javascript"));
     assert.equal(makeSnapshot("<Bar />", "typescriptreact"), "<Bar />");
     assert.equal(makeSnapshot("<Bar />", "javascriptreact"), "<Bar />");
     assert.equal(makeSnapshot("<Bar />", "flow"), "<Bar />");