add a new lint for preferring throw over return ThrowCompletion(...) (#692)

Author: michaelficarra

src/lint/collect-algorithm-diagnostics.ts

@@ -14,6 +14,7 @@ import lintForEachElement from './rules/for-each-element';
 import lintForEachOf from './rules/for-each-of';
 import lintStepAttributes from './rules/step-attributes';
 import lintIfElseConsistency from './rules/if-else-consistency';
+import lintPreferThrowShorthand from './rules/prefer-throw-shorthand';
 import lintAlgorithmSpelling from './rules/algorithm-spelling';
 import { checkVariableUsage } from './rules/variable-use-def';
 import type { Seq } from '../expr-parser';
@@ -35,6 +36,7 @@ const stepRules: LineRule[] = [
   lintForEachOf,
   lintStepAttributes,
   lintIfElseConsistency,
+  lintPreferThrowShorthand,
   lintAlgorithmSpelling,
 ];
 

src/lint/rules/prefer-throw-shorthand.ts

@@ -0,0 +1,41 @@
+import type { Reporter } from '../algorithm-error-reporter-type';
+import type { OrderedListItemNode } from 'ecmarkdown';
+import type { Seq } from '../../expr-parser';
+import { offsetToLineAndColumn } from '../../utils';
+
+const ruleId = 'prefer-throw-shorthand';
+
+/*
+Checks that "return ThrowCompletion(...)" is written using the "throw" shorthand.
+Does not flag ThrowCompletion when its result is assigned to an alias.
+*/
+export default function (
+  report: Reporter,
+  step: OrderedListItemNode,
+  algorithmSource: string,
+  parsedSteps: Map<OrderedListItemNode, Seq>,
+) {
+  const stepSeq = parsedSteps.get(step);
+  if (stepSeq == null) {
+    return;
+  }
+  const items = stepSeq.items;
+  for (let i = 1; i < items.length; ++i) {
+    const item = items[i];
+    if (
+      item.name === 'call' &&
+      item.callee.length === 1 &&
+      item.callee[0].name === 'text' &&
+      item.callee[0].contents === 'ThrowCompletion'
+    ) {
+      const prev = items[i - 1];
+      if (prev.name === 'text' && /\breturn\s+$/i.test(prev.contents)) {
+        report({
+          ruleId,
+          message: 'prefer "throw _x_" over "return ThrowCompletion(_x_)"',
+          ...offsetToLineAndColumn(algorithmSource, item.callee[0].location.start.offset),
+        });
+      }
+    }
+  }
+}

test/lint-prefer-throw-shorthand.ts

@@ -0,0 +1,83 @@
+import { describe, it, before } from 'node:test';
+import {
+  assertLint,
+  assertLintFree,
+  lintLocationMarker as M,
+  positioned,
+  getBiblio,
+} from './utils.ts';
+import type { ExportedBiblio } from '../lib/Biblio.js';
+
+const nodeType = 'emu-alg';
+
+describe('prefer-throw-shorthand', () => {
+  let throwBiblio: ExportedBiblio;
+  before(async () => {
+    throwBiblio = await getBiblio(`
+      <emu-clause id="sec-throwcompletion" type="abstract operation">
+        <h1>
+          ThrowCompletion (
+            _value_: an ECMAScript language value,
+          ): a throw completion
+        </h1>
+        <dl class="header">
+        </dl>
+        <emu-alg>
+          1. Return Completion Record { [[Type]]: ~throw~, [[Value]]: _value_, [[Target]]: ~empty~ }.
+        </emu-alg>
+      </emu-clause>
+    `);
+  });
+
+  it('return ThrowCompletion', async () => {
+    await assertLint(
+      positioned`<emu-alg>
+        1. Let _x_ be *"foo"*.
+        1. Return ${M}ThrowCompletion(_x_).
+      </emu-alg>`,
+      {
+        ruleId: 'prefer-throw-shorthand',
+        nodeType,
+        message: 'prefer "throw _x_" over "return ThrowCompletion(_x_)"',
+      },
+      { extraBiblios: [throwBiblio] },
+    );
+  });
+
+  it('return ThrowCompletion in single-line If', async () => {
+    await assertLint(
+      positioned`<emu-alg>
+        1. Let _x_ be *"foo"*.
+        1. If _x_ is *"bar"*, return ${M}ThrowCompletion(_x_).
+      </emu-alg>`,
+      {
+        ruleId: 'prefer-throw-shorthand',
+        nodeType,
+        message: 'prefer "throw _x_" over "return ThrowCompletion(_x_)"',
+      },
+      { extraBiblios: [throwBiblio] },
+    );
+  });
+
+  it('throw shorthand is allowed', async () => {
+    await assertLintFree(`
+      <emu-alg>
+        1. Let _x_ be *"foo"*.
+        1. Throw _x_.
+      </emu-alg>
+    `);
+  });
+
+  it('ThrowCompletion assigned to alias is allowed', async () => {
+    await assertLintFree(
+      `
+      <emu-alg>
+        1. Let _x_ be *"foo"*.
+        1. Let _comp_ be ThrowCompletion(_x_).
+        1. Return _comp_.
+      </emu-alg>
+    `,
+      { extraBiblios: [throwBiblio] },
+    );
+  });
+});

test/typecheck.ts

@@ -191,7 +191,7 @@ describe('typechecking completions', () => {
           <dl class="header">
           </dl>
           <emu-alg>
-            1. If some condition holds, return ThrowCompletion(*null*).
+            1. If some condition holds, throw *null*.
             1. Else if some other condition holds, return ReturnCompletion(*null*).
             1. Return NormalCompletion(*null*).
           </emu-alg>