js-compiler: Add preferFunctionDeclarations lint for const function expressions
MODIFY
packages/@romejs/cli-reporter/Reporter.ts
packages/@romejs/cli-reporter/Reporter.ts
@@ -535,7 +535,7 @@ export default class Reporter {
selectedOptions.add(optionNames[0]);
}
- const boundActive = () => {
+ function boundActive() {
activeOption = Math.min(activeOption, optionCount - 1);
activeOption = Math.max(activeOption, 0);
@@ -543,7 +543,7 @@ export default class Reporter {
selectedOptions.clear();
selectedOptions.add(optionNames[activeOption]);
}
- };
+ }
// If we aren't a radio then set the active option to the bottom of any that are enabled
if (!radio) {

MODIFY
packages/@romejs/core/master/bundler/BundleRequest.ts
packages/@romejs/core/master/bundler/BundleRequest.ts
@@ -267,7 +267,7 @@ export default class BundleRequest {
*/
const declaredCJS: Set<DependencyNode> = new Set();
- const declareCJS = (module: DependencyNode) => {
+ function declareCJS(module: DependencyNode) {
if (
mode !== 'modern' ||
module.type !== 'cjs' ||
@@ -279,7 +279,7 @@ export default class BundleRequest {
declaredCJS.add(module);
push(` var ${getPrefixedBundleNamespace(module.id)} = {};`);
- };
+ }
// Add on files
for (const source of files) {

MODIFY
packages/@romejs/core/master/fs/MemoryFileSystem.ts
packages/@romejs/core/master/fs/MemoryFileSystem.ts
@@ -228,10 +228,10 @@ async function createWatchmanWatcher(
}
clearTimeout(timeout);
- const processChanges = async (
+ async function processChanges(
data: WatchmanSubscriptionValue,
diagnostics: DiagnosticsProcessor,
- ) => {
+ ) {
if (data['state-enter'] || data['state-leave']) {
return;
}
@@ -295,7 +295,7 @@ async function createWatchmanWatcher(
}
}),
);
- };
+ }
activity.setText(`Processing results`);
await processChanges(initial, diagnostics);

MODIFY
packages/@romejs/js-ast-utils/template.ts
packages/@romejs/js-ast-utils/template.ts
@@ -77,7 +77,7 @@ function getTemplate(strs: TemplateStringsArray): BuiltTemplate {
ast = program.assert(removeLoc(ast));
// traverse and find placeholders paths
- const collectPlaceholderPaths = (path: Path) => {
+ function collectPlaceholderPaths(path: Path) {
const {node} = path;
if (isIdentifierish(node) && node.name in placeholders) {
placeholders[node.name] = {
@@ -86,7 +86,7 @@ function getTemplate(strs: TemplateStringsArray): BuiltTemplate {
};
}
return node;
- };
+ }
const context = new Context({
ast,
project: {

MODIFY
packages/@romejs/js-ast/statements/ReturnStatement.ts
packages/@romejs/js-ast/statements/ReturnStatement.ts
*/
import {JSNodeBase, AnyExpression} from '../index';
-import {createBuilder} from '../utils';
+import {createQuickBuilder} from '../utils';
export type ReturnStatement = JSNodeBase & {
type: 'ReturnStatement';
argument?: AnyExpression;
};
-export const returnStatement = createBuilder<ReturnStatement>(
+export const returnStatement = createQuickBuilder<ReturnStatement, 'argument'>(
'ReturnStatement',
+ 'argument',
{
bindingKeys: {},
visitorKeys: {

MODIFY
packages/@romejs/js-ast/utils.ts
packages/@romejs/js-ast/utils.ts
@@ -131,13 +131,13 @@ class Builder<Node extends AnyNode> {
}
}
- assert(res: TransformExitResult): Node {
- const node = assertSingleNode(res);
-
- if (node === undefined) {
+ assert(res: undefined | TransformExitResult): Node {
+ if (res === undefined) {
throw new Error(`Expected ${this.type} Node but got undefined`);
}
+ const node = assertSingleNode(res);
+
if (node.type !== this.type) {
throw new Error(`Expected ${this.type} Node but got ${node.type}`);
}

MODIFY
packages/@romejs/js-compiler/__rtests__/__rsnapshots__/lint.ts.rsnap
packages/@romejs/js-compiler/__rtests__/__rsnapshots__/lint.ts.rsnap
"no async promise executor": "Object {\n filters: Array []\n src: 'new Promise(async function foo() {})'\n diagnostics: Array [\n Object {\n category: 'lint/noAsyncPromiseExecutor'\n filename: 'unknown'\n language: 'js'\n message: 'Promise executor functions should not be async.'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 35\n index: 35\n line: 1\n }\n start: Object {\n column: 12\n index: 12\n line: 1\n }\n }\n ]\n}",
"no cond assign: 3": "Object {\n filters: Array []\n src: 'do {\\n console.log(\\'foo)\\n } while (foo = \\'bar\\')'\n diagnostics: Array [\n Object {\n advice: undefined\n category: 'parse/js'\n filename: 'unknown'\n message: 'Unterminated string constant'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'js-parser'}]\n end: Object {\n column: 0\n index: 28\n line: 3\n }\n start: Object {\n column: 0\n index: 28\n line: 3\n }\n }\n Object {\n category: 'lint/undeclaredVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Undeclared variable <emphasis>INVALID_PLACEHOLDER</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 50\n index: 55\n line: 2\n }\n start: Object {\n column: 50\n index: 55\n line: 2\n }\n }\n Object {\n category: 'lint/undeclaredVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Undeclared variable <emphasis>bar</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 47\n index: 52\n line: 2\n }\n start: Object {\n column: 44\n index: 49\n line: 2\n }\n }\n ]\n}",
"no async promise executor: 1": "Object {\n filters: Array []\n src: 'new Promise(async () => {})'\n diagnostics: Array [\n Object {\n category: 'lint/noAsyncPromiseExecutor'\n filename: 'unknown'\n language: 'js'\n message: 'Promise executor functions should not be async.'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 26\n index: 26\n line: 1\n }\n start: Object {\n column: 12\n index: 12\n line: 1\n }\n }\n ]\n}",
- "no async promise executor: 2": "Object {\n filters: Array []\n src: 'new Promise(((((async () => {})))))'\n diagnostics: Array [\n Object {\n category: 'lint/noAsyncPromiseExecutor'\n filename: 'unknown'\n language: 'js'\n message: 'Promise executor functions should not be async.'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 30\n index: 30\n line: 1\n }\n start: Object {\n column: 16\n index: 16\n line: 1\n }\n }\n ]\n}"
+ "no async promise executor: 2": "Object {\n filters: Array []\n src: 'new Promise(((((async () => {})))))'\n diagnostics: Array [\n Object {\n category: 'lint/noAsyncPromiseExecutor'\n filename: 'unknown'\n language: 'js'\n message: 'Promise executor functions should not be async.'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 30\n index: 30\n line: 1\n }\n start: Object {\n column: 16\n index: 16\n line: 1\n }\n }\n ]\n}",
+ "prefer function declarations": "Object {\n filters: Array []\n src: 'function foo() {}\\n'\n diagnostics: Array [\n Object {\n category: 'lint/preferFunctionDeclarations'\n filename: 'unknown'\n fixable: true\n language: 'js'\n message: 'Use a function declaration instead of a const function'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 26\n index: 26\n line: 1\n }\n start: Object {\n column: 12\n index: 12\n line: 1\n }\n }\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused variable <emphasis>foo</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 9\n index: 9\n line: 1\n }\n start: Object {\n column: 6\n index: 6\n line: 1\n }\n }\n ]\n}",
+ "prefer function declarations: 1": "Object {\n filters: Array []\n src: 'function foo() {}\\n'\n diagnostics: Array [\n Object {\n category: 'lint/preferFunctionDeclarations'\n filename: 'unknown'\n fixable: true\n language: 'js'\n message: 'Use a function declaration instead of a const function'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 20\n index: 20\n line: 1\n }\n start: Object {\n column: 12\n index: 12\n line: 1\n }\n }\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused variable <emphasis>foo</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 9\n index: 9\n line: 1\n }\n start: Object {\n column: 6\n index: 6\n line: 1\n }\n }\n ]\n}",
+ "prefer function declarations: 2": "Object {\n filters: Array []\n src: 'const foo = () => {\\n this;\\n};\\n'\n diagnostics: Array [\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused variable <emphasis>foo</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 9\n index: 9\n line: 1\n }\n start: Object {\n column: 6\n index: 6\n line: 1\n }\n }\n ]\n}",
+ "prefer function declarations: 3": "Object {\n filters: Array []\n src: 'function foo() {\\n function bar() {\\n this;\\n }\\n}\\n'\n diagnostics: Array [\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused function <emphasis>bar</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 31\n index: 31\n line: 1\n }\n start: Object {\n column: 28\n index: 28\n line: 1\n }\n }\n Object {\n category: 'lint/preferFunctionDeclarations'\n filename: 'unknown'\n fixable: true\n language: 'js'\n message: 'Use a function declaration instead of a const function'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 42\n index: 42\n line: 1\n }\n start: Object {\n column: 12\n index: 12\n line: 1\n }\n }\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused variable <emphasis>foo</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 9\n index: 9\n line: 1\n }\n start: Object {\n column: 6\n index: 6\n line: 1\n }\n }\n ]\n}",
+ "prefer function declarations: 4": "Object {\n filters: Array []\n src: 'const foo any = function() {};\\n'\n diagnostics: Array [\n Object {\n category: 'parse/js'\n filename: 'unknown'\n message: 'A type annotation is only valid inside of a TypeScript or Flow file'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'js-parser'}]\n end: Object {\n column: 9\n index: 9\n line: 1\n }\n start: Object {\n column: 9\n index: 9\n line: 1\n }\n advice: Array [\n log {\n category: 'info'\n message: 'Did you mean <emphasis>TypeScript</emphasis>? Change the file extension to <emphasis>.ts</emphasis> or <emphasis>.tsx</emphasis>'\n }\n log {\n category: 'info'\n message: 'Did you mean <emphasis>Flow</emphasis>? Add a <emphasis>@flow</emphasis> comment annotation to the top of the file'\n }\n ]\n }\n Object {\n category: 'lint/unusedVariables'\n filename: 'unknown'\n language: 'js'\n message: 'Unused variable <emphasis>foo</emphasis>'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 14\n index: 14\n line: 1\n }\n start: Object {\n column: 6\n index: 6\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression": "Object {\n filters: Array []\n src: 'let foo = /^abc[]/;foo;'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 18\n index: 18\n line: 1\n }\n start: Object {\n column: 10\n index: 10\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression: 1": "Object {\n filters: Array []\n src: 'let foo = /foo[]bar/;foo;'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 20\n index: 20\n line: 1\n }\n start: Object {\n column: 10\n index: 10\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression: 2": "Object {\n filters: Array []\n src: 'let foo = \"\";if (foo.match(/^abc[]/)) { foo; }'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 35\n index: 35\n line: 1\n }\n start: Object {\n column: 27\n index: 27\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression: 3": "Object {\n filters: Array []\n src: 'let foo = /[]]/;foo;'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 15\n index: 15\n line: 1\n }\n start: Object {\n column: 10\n index: 10\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression: 4": "Object {\n filters: Array []\n src: 'let foo = /\\\\[[]/;foo;'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 16\n index: 16\n line: 1\n }\n start: Object {\n column: 10\n index: 10\n line: 1\n }\n }\n ]\n}",
+ "no empty character class in regular expression: 5": "Object {\n filters: Array []\n src: 'let foo = /\\\\[\\\\[\\\\]a-z[]/;foo;'\n diagnostics: Array [\n Object {\n category: 'lint/noEmptyCharacterClass'\n filename: 'unknown'\n language: 'js'\n message: 'Empty character classes in regular expressions are not allowed'\n mtime: undefined\n sourceType: 'module'\n origins: Array [Object {category: 'lint'}]\n end: Object {\n column: 23\n index: 23\n line: 1\n }\n start: Object {\n column: 10\n index: 10\n line: 1\n }\n }\n ]\n}"
}
\ No newline at end of file

MODIFY
packages/@romejs/js-compiler/__rtests__/lint.ts
packages/@romejs/js-compiler/__rtests__/lint.ts
@@ -736,3 +736,40 @@ test('no shadow restricted names', async t => {
}
}
});
+
+test('prefer function declarations', async t => {
+ // Should complain on these
+ t.snapshot(
+ await testLint(
+ 'const foo = function () {};',
+ LINT_AND_FORMAT_ENABLED_CONFIG,
+ ),
+ );
+ t.snapshot(
+ await testLint('const foo = () => {};', LINT_AND_FORMAT_ENABLED_CONFIG),
+ );
+
+ // Should allow arrow functions when they have this
+ t.snapshot(
+ await testLint(
+ 'const foo = () => {this;};',
+ LINT_AND_FORMAT_ENABLED_CONFIG,
+ ),
+ );
+
+ // But only if it refers to the actual arrow function
+ t.snapshot(
+ await testLint(
+ 'const foo = () => {function bar() {this;}};',
+ LINT_AND_FORMAT_ENABLED_CONFIG,
+ ),
+ );
+
+ // Should ignore functions with return types since you can't express that with a declaration
+ t.snapshot(
+ await testLint(
+ 'const foo: any = function () {};',
+ LINT_AND_FORMAT_ENABLED_CONFIG,
+ ),
+ );
+});

MODIFY
packages/@romejs/js-compiler/transforms/lint/index.ts
packages/@romejs/js-compiler/transforms/lint/index.ts
@@ -22,13 +22,14 @@ import noEmptyCharacterClass from './noEmptyCharacterClass';
import noFunctionAssign from './noFunctionAssign';
import noImportAssign from './noImportAssign';
import noLabelVar from './noLabelVar';
+import noShadowRestrictedNames from './noShadowRestrictedNames';
import noTemplateCurlyInString from './noTemplateCurlyInString';
import noUnsafeFinally from './noUnsafeFinally';
+import preferFunctionDeclarations from './preferFunctionDeclarations';
import sparseArray from './sparseArray';
import undeclaredVariables from './undeclaredVariables';
import unsafeNegation from './unsafeNegation';
import unusedVariables from './unusedVariables';
-import noShadowRestrictedNames from './noShadowRestrictedNames';
// Add transforms in alphabetical order.
export const lintTransforms = [
@@ -48,11 +49,12 @@ export const lintTransforms = [
noFunctionAssign,
noImportAssign,
noLabelVar,
+ noShadowRestrictedNames,
noTemplateCurlyInString,
noUnsafeFinally,
+ preferFunctionDeclarations,
sparseArray,
undeclaredVariables,
unsafeNegation,
unusedVariables,
- noShadowRestrictedNames,
];

ADD
packages/@romejs/js-compiler/transforms/lint/preferFunctionDeclarations.ts
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+import {Path, createHook} from '@romejs/js-compiler';
+import {
+ AnyNode,
+ VariableDeclarationStatement,
+ FunctionDeclaration,
+ VariableDeclarator,
+ variableDeclarator,
+ variableDeclarationStatement,
+ functionDeclaration,
+ bindingIdentifier,
+ returnStatement,
+ blockStatement,
+ ThisExpression,
+} from '@romejs/js-ast';
+import {isFunctionNode} from '@romejs/js-ast-utils';
+
+type State = {
+ declarators: Array<VariableDeclarator>;
+};
+
+type Arg = {
+ declarator: VariableDeclarator;
+ node: ThisExpression;
+};
+
+// This hook is created with a list of initial VariableDeclarators that contain functions we want to convert
+// We then remove any ArrowFunctionExpression VariableDeclarators that contain a valid ThisExpression
+const hook = createHook<State, Arg, ThisExpression>({
+ name: 'preferFunctionDeclarationsHook',
+
+ initialState: {
+ declarators: [],
+ },
+
+ call(path: Path, state: State, {declarator, node}: Arg) {
+ return {
+ bubble: !state.declarators.includes(declarator),
+ value: node,
+ state: {
+ declarators: state.declarators.filter(decl => decl !== declarator),
+ },
+ };
+ },
+
+ exit(
+ path: Path,
+ state: State,
+ ):
+ | VariableDeclarationStatement
+ | Array<VariableDeclarationStatement | FunctionDeclaration> {
+ const node = variableDeclarationStatement.assert(path.node);
+
+ // We may have invalidated all declarations
+ if (state.declarators.length === 0) {
+ return node;
+ }
+
+ const nodes: Array<VariableDeclarationStatement | FunctionDeclaration> = [];
+
+ const newNode: VariableDeclarationStatement = {
+ ...node,
+ declaration: {
+ ...node.declaration,
+ declarations: node.declaration.declarations.filter(
+ decl => !state.declarators.includes(decl),
+ ),
+ },
+ };
+
+ // We may have removed all the declarators
+ if (newNode.declaration.declarations.length > 0) {
+ nodes.push(newNode);
+ }
+
+ // Convert functions
+ for (const decl of state.declarators) {
+ const id = bindingIdentifier.assert(decl.id);
+ const {init} = decl;
+
+ if (
+ init === undefined ||
+ (init.type !== 'FunctionExpression' &&
+ init.type !== 'ArrowFunctionExpression')
+ ) {
+ throw new Error('Invalid declarator put into state');
+ }
+
+ path.context.addNodeDiagnostic(init, {
+ category: 'lint/preferFunctionDeclarations',
+ message: 'Use a function declaration instead of a const function',
+ fixable: true,
+ });
+
+ // Convert arrow function body if necessary
+ const body =
+ init.body.type === 'BlockStatement'
+ ? init.body
+ : blockStatement.create({
+ body: [returnStatement.quick(init.body)],
+ });
+
+ nodes.push(
+ functionDeclaration.create({
+ id,
+ head: init.head,
+ body,
+ }),
+ );
+ }
+
+ return nodes;
+ },
+});
+
+export default {
+ name: 'preferFunctionDeclarations',
+ enter(path: Path): AnyNode {
+ const {node} = path;
+
+ if (
+ node.type === 'VariableDeclarationStatement' &&
+ node.declaration.kind === 'const'
+ ) {
+ // Get all declarators that are function expressions, have no type annotation, and have a binding identifier id
+ const declarators = node.declaration.declarations.filter(decl => {
+ return (
+ decl.id.type === 'BindingIdentifier' &&
+ (decl.id.meta === undefined ||
+ decl.id.meta.typeAnnotation === undefined) &&
+ decl.init !== undefined &&
+ (decl.init.type === 'FunctionExpression' ||
+ decl.init.type === 'ArrowFunctionExpression')
+ );
+ });
+ if (declarators.length > 0) {
+ return path.provideHook(hook, {
+ declarators,
+ });
+ }
+ }
+
+ // If we have a `this` inside of an arrow function attached as a variable declarator then we should consider
+ // it valid
+ if (node.type === 'ThisExpression') {
+ // Try to find the arrow function owner, or stop if we get to another function
+ const func = path.findAncestry(path => {
+ if (path.node.type === 'ArrowFunctionExpression') {
+ return path.parent.type === 'VariableDeclarator';
+ }
+
+ if (isFunctionNode(path.node)) {
+ return true;
+ }
+
+ return false;
+ });
+
+ // We'll only return an ArrowFunctionExpression if it was inside of a VariableDeclarator
+ if (func !== undefined && func.node.type === 'ArrowFunctionExpression') {
+ return path.callHook(
+ hook,
+ {
+ declarator: variableDeclarator.assert(func.parent),
+ node,
+ },
+ node,
+ );
+ }
+ }
+
+ return node;
+ },
+};

MODIFY
packages/@romejs/v8/sourceMapManager.ts
packages/@romejs/v8/sourceMapManager.ts
@@ -250,10 +250,10 @@ export function addSourceMapFactory(
init();
let map: undefined | SourceMap;
- const factoryCapture = () => {
+ function factoryCapture() {
map = factory();
return map;
- };
+ }
factories.set(filename, factoryCapture);

js-compiler: Add preferFunctionDeclarations lint for const function expressions