From 193c147c6294f028a0d71d7fb2193626a53f2ea9 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Mon, 17 May 2021 09:43:28 +0300 Subject: [PATCH 01/18] Add tests for export-level-one rule --- .../tests/lib/rules/export-level-one.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js new file mode 100644 index 00000000..856d9d3c --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js @@ -0,0 +1,50 @@ +const { RuleTester } = require("eslint"); +const rule = require("../../../lib/rules/export-level-one.js"); + +const ruleTester = new RuleTester({ + parserOptions: { ecmaVersion: 2015, sourceType: "module" }, + env: { + es6: true, + }, +}); + +ruleTester.run("export-level-one", rule, { + valid: [ + { + code: "export class Header {}", + }, + { + code: "export default class Header {}", + }, + { + code: "export const FETCH_COUNT = 5;", + }, + { + code: "export function formatInput(){}", + }, + { + code: "export default function formatInput(){}", + }, + { + code: "const {value} = SOME_CONSTANT;", + } + ], + + invalid: [ + { + code: "class Header {}", + output: "export class Header {}", + errors: [{ rule: "export-level-one" }], + }, + { + code: "const FETCH_COUNT = 5;", + output: "export const FETCH_COUNT = 5;", + errors: [{ rule: "export-level-one" }], + }, + { + code: "function formatInput(){}", + output: "export function formatInput(){}", + errors: [{ rule: "export-level-one" }], + }, + ], +}); From 6f4c68a9da4b3d5c7a03accaf120a028029aec49 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Mon, 17 May 2021 09:54:58 +0300 Subject: [PATCH 02/18] Add documentation for the export-level-one rule --- .../docs/rules/export-level-one.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md new file mode 100644 index 00000000..b6bcae83 --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md @@ -0,0 +1,23 @@ +# export-level-one +In Scandi, all top-level declaration need to be [exported](https://javascript.info/import-export). This ensures that + your code remains extensible. This rule applies to all top-level class, function and constant declarations. + +Examples of **incorrect** code for this rule: + +```js +class Header {} + +const FETCH_COUNT = 5; + +function formatInput(){} +``` + +Examples of **correct** code for this rule: + +```js +export class Header {} + +export const FETCH_COUNT = 5; + +export function formatInput(){} +``` From 253f9534dd8b54fc68c19df7a7428533717595d0 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Mon, 17 May 2021 10:00:43 +0300 Subject: [PATCH 03/18] export-level-one rule: refactor and improve error message --- .../lib/rules/export-level-one.js | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js index ebde26b4..34809c30 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js @@ -3,29 +3,54 @@ * @author Jegors Batovs */ +const { constructMessage } = require('../util/messages'); + const CLASS_DECLARATION = 'ClassDeclaration'; const FUNCTION_DECLARATION = 'FunctionDeclaration'; const VARIABLE_DECLARATION = 'VariableDeclaration'; +const OBJECT_PATTERN = 'ObjectPattern'; -const shouldGetExported = [ +const exportableTypes = [ CLASS_DECLARATION, FUNCTION_DECLARATION, VARIABLE_DECLARATION, ]; -const shouldBeExported = (node) => { +const isDestructuringAssignment = (node) => { + const { type, declarations } = node; + return ( - node.type !== VARIABLE_DECLARATION || - !node.declarations.find((one) => one.id.type === 'ObjectPattern') + type === VARIABLE_DECLARATION && + declarations.some((declaration) => declaration.id.type === OBJECT_PATTERN) ); }; -const getName = (node) => { +const shouldBeExported = (node) => { + const { type } = node; + + if (!exportableTypes.includes(type)) { + return false; + } + + return !isDestructuringAssignment(node) +}; + +const getNameFromDeclaration = (node) => { if ([CLASS_DECLARATION, FUNCTION_DECLARATION].includes(node.type)) { return node.id.name; } - return 'This variable'; + return 'variable'; +}; + +const getExportErrorMessage = (exportable) => { + const name = getNameFromDeclaration(exportable); + + const error = 'In Scandi, all top-level declaration need to be exported. This ensures that your code remains extensible.'; + const help = `To fix this error, export the ${ name } declaration by adding "export" before it.`; + const documentationLink = 'https://github.com/scandipwa/eslint/blob/master/docs/rules/export-level-one.md'; + + return constructMessage(error, help, documentationLink); }; module.exports = { @@ -44,17 +69,13 @@ module.exports = { const { body } = node; body - .filter((levelOneNode) => shouldGetExported.includes(levelOneNode.type)) - .map((exportable) => { - if (shouldBeExported(exportable)) { - context.report({ - node: exportable, - message: `${getName( - exportable - )} must be exported (as non default) to allow proper extension`, - fix: (fixer) => fixer.insertTextBefore(exportable, 'export '), - }); - } + .filter((node) => shouldBeExported(node)) + .map((declarationNode) => { + context.report({ + node: declarationNode, + message: getExportErrorMessage(declarationNode), + fix: (fixer) => fixer.insertTextBefore(declarationNode, 'export '), + }); }); }, }), From b811896f35d2da62f1875ebdf50dad3f954c41fe Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Tue, 25 May 2021 16:22:47 +0300 Subject: [PATCH 04/18] Fix unit tests to work with current eslint version --- .../tests/lib/rules/export-level-one.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js index 856d9d3c..a61e9977 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/export-level-one.js @@ -34,17 +34,17 @@ ruleTester.run("export-level-one", rule, { { code: "class Header {}", output: "export class Header {}", - errors: [{ rule: "export-level-one" }], + errors: 1, }, { code: "const FETCH_COUNT = 5;", output: "export const FETCH_COUNT = 5;", - errors: [{ rule: "export-level-one" }], + errors: 1, }, { code: "function formatInput(){}", output: "export function formatInput(){}", - errors: [{ rule: "export-level-one" }], + errors: 1, }, ], }); From e6106e6acada393292a24ecf30153d53ccf55f7d Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Tue, 25 May 2021 16:23:18 +0300 Subject: [PATCH 05/18] Update docs for derived-class-names.md and export-level-one.md --- .../docs/rules/derived-class-names.md | 26 +++++++++---------- .../docs/rules/export-level-one.md | 5 ++++ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md index cfe5b57c..632cd958 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md @@ -1,27 +1,27 @@ # Class name must match the name of the file it is declared in. (derived-class-names) -Expected class names for all the files **other than components** are `name + prefix` (e.g. class inside of `AddToCart.container.js` file must be called `AddToCartContainer` and not otherwise). - -## Rule Details - -Notice, that this rule is not applied to `component` postfix. +Class names need to be based on the file name. For example, a class declared in `Footer.component.js` must be named + `FooterComponent`. Examples of **incorrect** code for this rule: ```js -// in MyComponent.container.js -class Abc { /** ... */ } +// in Goodbye.component.js +class HelloComponent {} -// in Hello.component.js -class HelloComponent { /** ... */ } +// in Footer.container.js +class FooterComponent {} ``` Examples of **correct** code for this rule: ```js -// in MyComponent.container.js -class MyComponentContainer { /** ... */ } +// in Footer.component.js +class FooterComponent {} -// in Hello.component.js -class Hello { /** ... */ } +// in Footer.container.js +class FooterContainer {} ``` + +## Why? +Naming classes according to the filename helps keep class names consistent and predictable across the codebase. diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md index b6bcae83..8b16a72a 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md @@ -21,3 +21,8 @@ export const FETCH_COUNT = 5; export function formatInput(){} ``` + +# Why? +Exporting all declarations ensures that your code is easily extensible using the +[Override Mechanism](https://docs.scandipwa.com/developing-with-scandi/override-mechanism). This rule helps ensure + that your code will work well with the rest of the Scandi ecosystem. From cfc66408d59c7fd9ea85176cd5d2b3f3358c5454 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Tue, 25 May 2021 16:24:51 +0300 Subject: [PATCH 06/18] Fix typo --- .../lib/rules/export-level-one.js | 3 +- yarn.lock | 58 ------------------- 2 files changed, 2 insertions(+), 59 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js index 34809c30..606266fd 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js @@ -46,7 +46,8 @@ const getNameFromDeclaration = (node) => { const getExportErrorMessage = (exportable) => { const name = getNameFromDeclaration(exportable); - const error = 'In Scandi, all top-level declaration need to be exported. This ensures that your code remains extensible.'; + const error = 'In Scandi, all top-level declarations need to be exported. This ensures that your code remains' + + ' extensible.'; const help = `To fix this error, export the ${ name } declaration by adding "export" before it.`; const documentationLink = 'https://github.com/scandipwa/eslint/blob/master/docs/rules/export-level-one.md'; diff --git a/yarn.lock b/yarn.lock index 1b819ec5..a2f4e6ec 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1205,21 +1205,6 @@ minimatch "^3.0.4" strip-json-comments "^3.1.1" -"@eslint/eslintrc@^0.4.1": - version "0.4.1" - resolved "https://registry.yarnpkg.com/@eslint/eslintrc/-/eslintrc-0.4.1.tgz#442763b88cecbe3ee0ec7ca6d6dd6168550cbf14" - integrity sha512-5v7TDE9plVhvxQeWLXDTvFvJBdH6pEsdnl2g/dAptmuFEPedQ4Erq5rsDsX+mvAM610IhNaO2W5V1dOOnDKxkQ== - dependencies: - ajv "^6.12.4" - debug "^4.1.1" - espree "^7.3.0" - globals "^12.1.0" - ignore "^4.0.6" - import-fresh "^3.2.1" - js-yaml "^3.13.1" - minimatch "^3.0.4" - strip-json-comments "^3.1.1" - "@evocateur/libnpmaccess@^3.1.2": version "3.1.2" resolved "https://registry.yarnpkg.com/@evocateur/libnpmaccess/-/libnpmaccess-3.1.2.tgz#ecf7f6ce6b004e9f942b098d92200be4a4b1c845" @@ -6516,49 +6501,6 @@ eslint@^7.11.0, eslint@^7.19.0: text-table "^0.2.0" v8-compile-cache "^2.0.3" -eslint@^7.26.0: - version "7.26.0" - resolved "https://registry.yarnpkg.com/eslint/-/eslint-7.26.0.tgz#d416fdcdcb3236cd8f282065312813f8c13982f6" - integrity sha512-4R1ieRf52/izcZE7AlLy56uIHHDLT74Yzz2Iv2l6kDaYvEu9x+wMB5dZArVL8SYGXSYV2YAg70FcW5Y5nGGNIg== - dependencies: - "@babel/code-frame" "7.12.11" - "@eslint/eslintrc" "^0.4.1" - ajv "^6.10.0" - chalk "^4.0.0" - cross-spawn "^7.0.2" - debug "^4.0.1" - doctrine "^3.0.0" - enquirer "^2.3.5" - eslint-scope "^5.1.1" - eslint-utils "^2.1.0" - eslint-visitor-keys "^2.0.0" - espree "^7.3.1" - esquery "^1.4.0" - esutils "^2.0.2" - file-entry-cache "^6.0.1" - functional-red-black-tree "^1.0.1" - glob-parent "^5.0.0" - globals "^13.6.0" - ignore "^4.0.6" - import-fresh "^3.0.0" - imurmurhash "^0.1.4" - is-glob "^4.0.0" - js-yaml "^3.13.1" - json-stable-stringify-without-jsonify "^1.0.1" - levn "^0.4.1" - lodash "^4.17.21" - minimatch "^3.0.4" - natural-compare "^1.4.0" - optionator "^0.9.1" - progress "^2.0.0" - regexpp "^3.1.0" - semver "^7.2.1" - strip-ansi "^6.0.0" - strip-json-comments "^3.1.0" - table "^6.0.4" - text-table "^0.2.0" - v8-compile-cache "^2.0.3" - espree@^6.1.2: version "6.2.1" resolved "https://registry.yarnpkg.com/espree/-/espree-6.2.1.tgz#77fc72e1fd744a2052c20f38a5b575832e82734a" From 2b90796b9f9e32c85d5c3985533966052302726e Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Tue, 25 May 2021 16:38:01 +0300 Subject: [PATCH 07/18] Specify documentation link in metadata --- .../lib/rules/export-level-one.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js index 606266fd..14855517 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js @@ -10,6 +10,8 @@ const FUNCTION_DECLARATION = 'FunctionDeclaration'; const VARIABLE_DECLARATION = 'VariableDeclaration'; const OBJECT_PATTERN = 'ObjectPattern'; +const DOCUMENTATION_URL = 'https://github.com/scandipwa/eslint/blob/master/docs/rules/export-level-one.md'; + const exportableTypes = [ CLASS_DECLARATION, FUNCTION_DECLARATION, @@ -49,9 +51,8 @@ const getExportErrorMessage = (exportable) => { const error = 'In Scandi, all top-level declarations need to be exported. This ensures that your code remains' + ' extensible.'; const help = `To fix this error, export the ${ name } declaration by adding "export" before it.`; - const documentationLink = 'https://github.com/scandipwa/eslint/blob/master/docs/rules/export-level-one.md'; - return constructMessage(error, help, documentationLink); + return constructMessage(error, help, DOCUMENTATION_URL); }; module.exports = { @@ -61,6 +62,7 @@ module.exports = { 'Everything declared in module on the first nesting level should be exported.', category: 'Coding standard', recommended: false, + url: DOCUMENTATION_URL, }, fixable: 'code', }, From 3192530e7995eabe1d894a290b034ba3330769ac Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 26 May 2021 09:51:50 +0300 Subject: [PATCH 08/18] Add basic test for use-namespace rule; fix rule to work with tests; fix eslint compatibility bug --- .../lib/rules/use-namespace.js | 30 +++++++++++++++++-- .../lib/util/fix-namespace-lack.js | 2 +- .../tests/lib/rules/use-namespace.js | 26 ++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js index 9d65bccd..4f0089fa 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js @@ -163,11 +163,35 @@ const preparePackageName = (packageName) => { return `${org.slice(1)}/${name}`; }; -const generateNamespace = (node, context) => { +const getPackageName = (context) => { const filePath = context.getFilename(); + + // if we are in a unit test, mock the package name + if (filePath === "") { + return 'TestPackage'; + } + const modulePath = walkDirectoryUp(filePath).pathname; - const fileRelative = path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); - const { name: packageName } = getPackageJson(modulePath); + const { name } = getPackageJson(modulePath); + + return name; +} + +const getPackageRelativePath = (context) => { + const filePath = context.getFilename(); + + // if we are in a unit test, mock the package name + if (filePath === "") { + return 'test/path'; + } + + const modulePath = walkDirectoryUp(filePath).pathname; + return path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); +} + +const generateNamespace = (node, context) => { + const packageName = getPackageName(context); + const fileRelative = getPackageRelativePath(context); // Not using path.join to support windows const pathname = [ diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js index de26b2c7..52d236f0 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js @@ -37,7 +37,7 @@ module.exports = (fixer, node, context, namespace) => { return fixer.insertTextBefore( node, `${ - context.getSourceCode().text[node.start - 1] === '(' ? '\n' : '' + context.getSourceCode().text[node.loc.start - 1] === '(' ? '\n' : '' }/** @namespace ${namespace} */\n` ); } diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js new file mode 100644 index 00000000..30d8fedf --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js @@ -0,0 +1,26 @@ +const { RuleTester } = require("eslint"); +const rule = require("../../../lib/rules/use-namespace.js"); + +const ruleTester = new RuleTester({ + parserOptions: { ecmaVersion: 2015, sourceType: "module" }, + env: { + es6: true, + }, +}); + +ruleTester.run("use-namespace", rule, { + valid: [ + { + code: "/** @namespace TestPackage/Test/Path/HeaderComponent */ export class HeaderComponent {}", + }, + ], + + invalid: [ + { + code: "export class HeaderComponent {}", + output: "/** @namespace TestPackage/Test/Path/HeaderComponent */\n" + + "export class HeaderComponent {}", + errors: 1, + }, + ], +}); From 46c9bc784eb217f2b2c47d1aac42189b087a4ab6 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 26 May 2021 10:00:04 +0300 Subject: [PATCH 09/18] Additional tests for use-namespace --- .../tests/lib/rules/use-namespace.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js index 30d8fedf..e4f0da88 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js @@ -22,5 +22,33 @@ ruleTester.run("use-namespace", rule, { "export class HeaderComponent {}", errors: 1, }, + { + code: "/** @namespace The/Wrong/Namespace/HeaderComponent */ export class HeaderComponent {}", + output: "/** @namespace TestPackage/Test/Path/HeaderComponent */ export class HeaderComponent {}", + errors: 1, + }, + { + code: "/** @namespace TestPackage/Test/Path/WrongExportName */ export class HeaderComponent {}", + output: "/** @namespace TestPackage/Test/Path/HeaderComponent */ export class HeaderComponent {}", + errors: 1, + }, + { + code: "/**\n" + + "Some documentation comment\n" + + "*/\n" + + "export class HeaderComponent {}", + output: "/**\n" + + "Some documentation comment\n" + + "* @namespace TestPackage/Test/Path/HeaderComponent\n" + + " */\n" + + "export class HeaderComponent {}", + errors: 1, + }, + { + code: "export const mapStateToProps = () => {};", + output: "/** @namespace TestPackage/Test/Path/mapStateToProps */\n" + + "export const mapStateToProps = () => {};", + errors: 1, + }, ], }); From d3c689f0dfd8ddfaca86afa29085b0469acb7a64 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 26 May 2021 10:25:23 +0300 Subject: [PATCH 10/18] Cleanup code --- .../lib/rules/use-namespace.js | 176 +++++++++--------- .../lib/util/fix-namespace-lack.js | 2 +- 2 files changed, 87 insertions(+), 91 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js index 4f0089fa..83c19b2e 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js @@ -12,14 +12,14 @@ const { walkDirectoryUp } = require('@scandipwa/scandipwa-dev-utils/get-context' const types = { ExportedClass: [ 'ExportNamedDeclaration', - 'ClassDeclaration' + 'ClassDeclaration', ].join(' > '), ExportedArrowFunction: [ 'ExportNamedDeclaration', 'VariableDeclaration', 'VariableDeclarator', - 'ArrowFunctionExpression' + 'ArrowFunctionExpression', ].join(' > '), isExportedClass: node => node.type === 'ClassDeclaration' @@ -32,15 +32,15 @@ const types = { PromiseHandlerArrowFunction: [ [ - "CallExpression", - "[callee.type='MemberExpression']", - "[callee.object.name!=/.+Dispatcher/]", - ":matches(", - "[callee.property.name='then'], ", - "[callee.property.name='catch']", - ")", + "CallExpression", + "[callee.type='MemberExpression']", + "[callee.object.name!=/.+Dispatcher/]", + ":matches(", + "[callee.property.name='then'], ", + "[callee.property.name='catch']", + ")", ].join(''), - 'ArrowFunctionExpression' + 'ArrowFunctionExpression', ].join(' > '), isPromiseHandlerArrowFunction: (node) => { @@ -49,23 +49,28 @@ const types = { return ( node.type === 'ArrowFunctionExpression' - && parent.type === 'CallExpression' - && parent.callee.type === 'MemberExpression' - && !(parent.callee.object.name || "").endsWith('Dispatcher') - && promiseHandlerNames.includes(parent.callee.property.name) + && parent.type === 'CallExpression' + && parent.callee.type === 'MemberExpression' + && !(parent.callee.object.name || "").endsWith('Dispatcher') + && promiseHandlerNames.includes(parent.callee.property.name) ); }, - isHandleableArrowFunction: node => types.isExportedArrowFunction(node) - || types.isPromiseHandlerArrowFunction(node), - - detectType: node => { + getNodeDescription: node => { if (types.isPromiseHandlerArrowFunction(node)) return 'promise handler arrow function'; if (types.isExportedArrowFunction(node)) return 'exported arrow function'; if (types.isExportedClass(node)) return 'exported class'; - } + + throw new Error("Unexpected node; could not provide description", node) + }, }; +const NAMESPACEABLE_NODE = [ + types.ExportedClass, + types.PromiseHandlerArrowFunction, + types.ExportedArrowFunction, +].join(','); + const getProperParentNode = (node) => { if (types.isExportedClass(node)) { return node.parent; @@ -81,13 +86,11 @@ const getProperParentNode = (node) => { }; const getNamespaceCommentForNode = (node, sourceCode) => { - const getNamespaceFromComments = (comments = []) => comments.find( - comment => comment.value.includes('@namespace') - ); - - return getNamespaceFromComments( - getLeadingCommentsForNode(getProperParentNode(node), sourceCode) - ); + const parent = getProperParentNode(node); + return getLeadingCommentsForNode(parent, sourceCode) + .find( + comment => comment.value.includes('@namespace'), + ); }; const collectFunctionNamespace = (node, stack) => { @@ -99,7 +102,7 @@ const collectFunctionNamespace = (node, stack) => { stack.push(node.callee.name); } } -} +}; const getNodeNamespace = (node) => { const stack = []; @@ -114,32 +117,30 @@ const getNodeNamespace = (node) => { // not using path.sep on purpose return stack.reverse().join('/'); -} +}; const prepareFilePath = (pathname) => { const { name: filename, - dir + dir, } = path.parse(pathname); - const [name, postfix = ''] = filename.split('.'); + const [name, postfix = ''] = filename.split('.'); /** * We do not want the \\ paths on Windows, rather / => * split and then join with correct delimiter **/ return path.join( - dir, - // If dir name === file name without postfix => do not repeat it - new RegExp(`${path.sep}${name}$`).test(dir) ? '' : name, - postfix + dir, + // If dir name === file name without postfix => do not repeat it + new RegExp(`${ path.sep }${ name }$`).test(dir) ? '' : name, + postfix, ).split(path.sep) - // Filter out empty strings if they exist - .filter(x => !!x); -} + .filter(x => x.length > 0); +}; -const preparePackageName = (packageName) => { - // This is on purpose not a path.sep (windows support) +const formatPackageName = (packageName) => { const [org = '', name = ''] = packageName.split('/'); if (!name) { @@ -160,14 +161,14 @@ const preparePackageName = (packageName) => { return name; } - return `${org.slice(1)}/${name}`; + return `${ org.slice(1) }/${ name }`; }; const getPackageName = (context) => { const filePath = context.getFilename(); // if we are in a unit test, mock the package name - if (filePath === "") { + if (filePath === "") { return 'TestPackage'; } @@ -175,48 +176,47 @@ const getPackageName = (context) => { const { name } = getPackageJson(modulePath); return name; -} +}; const getPackageRelativePath = (context) => { const filePath = context.getFilename(); - // if we are in a unit test, mock the package name - if (filePath === "") { + // if we are in a unit test, mock the relative path + if (filePath === "") { return 'test/path'; } const modulePath = walkDirectoryUp(filePath).pathname; return path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); -} +}; -const generateNamespace = (node, context) => { +const getExpectedNamespace = (node, context) => { const packageName = getPackageName(context); const fileRelative = getPackageRelativePath(context); // Not using path.join to support windows const pathname = [ - // remove @ from organization, support @scandipwa legacy namespaces - preparePackageName(packageName), + formatPackageName(packageName), // Trim post-fixes if they are not present - ...prepareFilePath(fileRelative) + ...prepareFilePath(fileRelative), ].join('/').replace( // Convert to pascal-case, and trim "-" /\b[a-z](?=[a-z]{2})/g, - (letter) => letter.toUpperCase() + (letter) => letter.toUpperCase(), ).split('-').join(''); // Do not transform code to uppercase / lowercase it should be written alright - return `${pathname}/${getNodeNamespace(node)}`; -} + return `${ pathname }/${ getNodeNamespace(node) }`; +}; -const extractNamespaceFromComment = ({ value: comment = '' }) => { - const { - groups: { - namespace - } = {} - } = comment.match(/@namespace +(?[^ ]+)/) || {}; +const getActualNamespace = ({ value: comment = '' }) => { + const { + groups: { + namespace, + } = {}, + } = comment.match(/@namespace +(?[^ ]+)/) || {}; - return namespace; + return namespace; }; module.exports = { @@ -224,51 +224,47 @@ module.exports = { docs: { description: 'Use @namespace comment-decorators', category: 'Extensibility', - recommended: true + recommended: true, }, - fixable: 'code' + fixable: 'code', }, create: context => ({ - [[ - types.ExportedClass, - types.PromiseHandlerArrowFunction, - types.ExportedArrowFunction - ].join(',')](node) { + [NAMESPACEABLE_NODE]: (node) => { const namespaceComment = getNamespaceCommentForNode(node, context.getSourceCode()) || { value: '' }; const namespaceCommentString = namespaceComment.value.split('@namespace').pop().trim(); - const namespace = extractNamespaceFromComment(namespaceComment); - const generatedNamespace = generateNamespace(node, context); + const actualNamespace = getActualNamespace(namespaceComment); + const expectedNamespace = getExpectedNamespace(node, context); if (!namespaceCommentString) { context.report({ node, - message: `Provide namespace for ${types.detectType(node)} by using @namespace magic comment`, + message: `Provide namespace for ${ types.getNodeDescription(node) } by using @namespace magic comment`, fix: fixer => fixNamespaceLack( - fixer, - getProperParentNode(node), - context, - generatedNamespace - ) || [] + fixer, + getProperParentNode(node), + context, + expectedNamespace, + ) || [], }); - } else if (generatedNamespace !== namespaceCommentString) { - context.report({ + } else if (expectedNamespace !== namespaceCommentString) { + context.report({ node, - message: `Namespace for this node is not valid! Consider changing it to ${generatedNamespace}`, + message: `Namespace for this node is not valid! Consider changing it to ${ expectedNamespace }`, fix: fixer => { - const newNamespaceCommentContent = namespaceComment.value.replace(namespace, generatedNamespace); - const newNamespaceComment = namespaceComment.type === 'Block' - ? `/*${newNamespaceCommentContent}*/` - : `// ${newNamespaceCommentContent}`; - - return fixer.replaceText( - namespaceComment, - newNamespaceComment - ) - } + const newNamespaceCommentContent = namespaceComment.value.replace(actualNamespace, expectedNamespace); + const newNamespaceComment = namespaceComment.type === 'Block' + ? `/*${ newNamespaceCommentContent }*/` + : `// ${ newNamespaceCommentContent }`; + + return fixer.replaceText( + namespaceComment, + newNamespaceComment, + ) + }, }); - } - } - }) + } + }, + }), }; diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js index 52d236f0..02e77bd1 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js @@ -12,7 +12,7 @@ module.exports = (fixer, node, context, namespace) => { const leadingComments = getLeadingCommentsForNode(node, sourceCode); if (leadingComments.find(comment => comment.value.includes('@namespace'))) { - return null; + throw new Error("Could not add namespace: node already has a namespace comment") } const blockComment = leadingComments.reverse().find( From 63c0c3b596ffc6365d370e2c8a84304bdd42c23d Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 26 May 2021 10:30:19 +0300 Subject: [PATCH 11/18] Refactor: move namespace utility functions to separate file --- .../lib/rules/use-namespace.js | 224 +---------------- .../lib/util/namespace.js | 226 ++++++++++++++++++ 2 files changed, 235 insertions(+), 215 deletions(-) create mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/lib/util/namespace.js diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js index 83c19b2e..82207379 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js @@ -3,221 +3,15 @@ * @author Jegors Batovs */ -const path = require('path'); -const { getPackageJson } = require('@scandipwa/scandipwa-dev-utils/package-json'); -const fixNamespaceLack = require('../util/fix-namespace-lack.js'); -const getLeadingCommentsForNode = require('../util/get-leading-comments'); -const { walkDirectoryUp } = require('@scandipwa/scandipwa-dev-utils/get-context'); - -const types = { - ExportedClass: [ - 'ExportNamedDeclaration', - 'ClassDeclaration', - ].join(' > '), - - ExportedArrowFunction: [ - 'ExportNamedDeclaration', - 'VariableDeclaration', - 'VariableDeclarator', - 'ArrowFunctionExpression', - ].join(' > '), - - isExportedClass: node => node.type === 'ClassDeclaration' - && node.parent.type === 'ExportNamedDeclaration', - - isExportedArrowFunction: node => node.type === 'ArrowFunctionExpression' - && node.parent.type === 'VariableDeclarator' - && node.parent.parent.type === 'VariableDeclaration' - && node.parent.parent.parent.type === 'ExportNamedDeclaration', - - PromiseHandlerArrowFunction: [ - [ - "CallExpression", - "[callee.type='MemberExpression']", - "[callee.object.name!=/.+Dispatcher/]", - ":matches(", - "[callee.property.name='then'], ", - "[callee.property.name='catch']", - ")", - ].join(''), - 'ArrowFunctionExpression', - ].join(' > '), - - isPromiseHandlerArrowFunction: (node) => { - const { parent } = node; - const promiseHandlerNames = ['then', 'catch']; - - return ( - node.type === 'ArrowFunctionExpression' - && parent.type === 'CallExpression' - && parent.callee.type === 'MemberExpression' - && !(parent.callee.object.name || "").endsWith('Dispatcher') - && promiseHandlerNames.includes(parent.callee.property.name) - ); - }, - - getNodeDescription: node => { - if (types.isPromiseHandlerArrowFunction(node)) return 'promise handler arrow function'; - if (types.isExportedArrowFunction(node)) return 'exported arrow function'; - if (types.isExportedClass(node)) return 'exported class'; - - throw new Error("Unexpected node; could not provide description", node) - }, -}; - -const NAMESPACEABLE_NODE = [ - types.ExportedClass, - types.PromiseHandlerArrowFunction, - types.ExportedArrowFunction, -].join(','); - -const getProperParentNode = (node) => { - if (types.isExportedClass(node)) { - return node.parent; - } - if (types.isExportedArrowFunction(node)) { - return node.parent.parent.parent; - } - if (types.isPromiseHandlerArrowFunction(node)) { - return node; - } - - return {}; -}; - -const getNamespaceCommentForNode = (node, sourceCode) => { - const parent = getProperParentNode(node); - return getLeadingCommentsForNode(parent, sourceCode) - .find( - comment => comment.value.includes('@namespace'), - ); -}; - -const collectFunctionNamespace = (node, stack) => { - if (node.type === 'CallExpression') { - if (node.callee.type === 'MemberExpression') { - stack.push(node.callee.property.name); - collectFunctionNamespace(node.callee.object, stack); - } else if (node.callee.type === 'Identifier') { - stack.push(node.callee.name); - } - } -}; - -const getNodeNamespace = (node) => { - const stack = []; - - if (node.parent.type === 'VariableDeclarator') { - stack.push(node.parent.id.name) - } else if (node.type === 'ClassDeclaration') { - stack.push(node.id.name); - } else { - collectFunctionNamespace(node.parent, stack); - } - - // not using path.sep on purpose - return stack.reverse().join('/'); -}; - -const prepareFilePath = (pathname) => { - const { - name: filename, - dir, - } = path.parse(pathname); +const { + getNodeDescription, + getActualNamespace, + getExpectedNamespace, + getNamespaceCommentForNode, + getProperParentNode, NAMESPACEABLE_NODE, +} = require('../util/namespace.js'); - const [name, postfix = ''] = filename.split('.'); - - /** - * We do not want the \\ paths on Windows, rather / => - * split and then join with correct delimiter - **/ - return path.join( - dir, - // If dir name === file name without postfix => do not repeat it - new RegExp(`${ path.sep }${ name }$`).test(dir) ? '' : name, - postfix, - ).split(path.sep) - .filter(x => x.length > 0); -}; - -const formatPackageName = (packageName) => { - const [org = '', name = ''] = packageName.split('/'); - - if (!name) { - // if there is no name => there is not ORG - if (packageName === '<%= name %>') { - return 'placeholder' - } - - return packageName; - } - - if (org === '@scandipwa') { - // Legacy support - if (name === 'scandipwa') { - return ''; - } - - return name; - } - - return `${ org.slice(1) }/${ name }`; -}; - -const getPackageName = (context) => { - const filePath = context.getFilename(); - - // if we are in a unit test, mock the package name - if (filePath === "") { - return 'TestPackage'; - } - - const modulePath = walkDirectoryUp(filePath).pathname; - const { name } = getPackageJson(modulePath); - - return name; -}; - -const getPackageRelativePath = (context) => { - const filePath = context.getFilename(); - - // if we are in a unit test, mock the relative path - if (filePath === "") { - return 'test/path'; - } - - const modulePath = walkDirectoryUp(filePath).pathname; - return path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); -}; - -const getExpectedNamespace = (node, context) => { - const packageName = getPackageName(context); - const fileRelative = getPackageRelativePath(context); - - // Not using path.join to support windows - const pathname = [ - formatPackageName(packageName), - // Trim post-fixes if they are not present - ...prepareFilePath(fileRelative), - ].join('/').replace( - // Convert to pascal-case, and trim "-" - /\b[a-z](?=[a-z]{2})/g, - (letter) => letter.toUpperCase(), - ).split('-').join(''); - - // Do not transform code to uppercase / lowercase it should be written alright - return `${ pathname }/${ getNodeNamespace(node) }`; -}; - -const getActualNamespace = ({ value: comment = '' }) => { - const { - groups: { - namespace, - } = {}, - } = comment.match(/@namespace +(?[^ ]+)/) || {}; - - return namespace; -}; +const fixNamespaceLack = require('../util/fix-namespace-lack.js'); module.exports = { meta: { @@ -240,7 +34,7 @@ module.exports = { if (!namespaceCommentString) { context.report({ node, - message: `Provide namespace for ${ types.getNodeDescription(node) } by using @namespace magic comment`, + message: `Provide namespace for ${ getNodeDescription(node) } by using @namespace magic comment`, fix: fixer => fixNamespaceLack( fixer, getProperParentNode(node), diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/namespace.js new file mode 100644 index 00000000..523d8a6f --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/namespace.js @@ -0,0 +1,226 @@ +const path = require('path'); +const { getPackageJson } = require('@scandipwa/scandipwa-dev-utils/package-json'); +const fixNamespaceLack = require('../util/fix-namespace-lack.js'); +const getLeadingCommentsForNode = require('../util/get-leading-comments'); +const { walkDirectoryUp } = require('@scandipwa/scandipwa-dev-utils/get-context'); + +const types = { + ExportedClass: [ + 'ExportNamedDeclaration', + 'ClassDeclaration', + ].join(' > '), + + ExportedArrowFunction: [ + 'ExportNamedDeclaration', + 'VariableDeclaration', + 'VariableDeclarator', + 'ArrowFunctionExpression', + ].join(' > '), + + isExportedClass: node => node.type === 'ClassDeclaration' + && node.parent.type === 'ExportNamedDeclaration', + + isExportedArrowFunction: node => node.type === 'ArrowFunctionExpression' + && node.parent.type === 'VariableDeclarator' + && node.parent.parent.type === 'VariableDeclaration' + && node.parent.parent.parent.type === 'ExportNamedDeclaration', + + PromiseHandlerArrowFunction: [ + [ + "CallExpression", + "[callee.type='MemberExpression']", + "[callee.object.name!=/.+Dispatcher/]", + ":matches(", + "[callee.property.name='then'], ", + "[callee.property.name='catch']", + ")", + ].join(''), + 'ArrowFunctionExpression', + ].join(' > '), + + isPromiseHandlerArrowFunction: (node) => { + const { parent } = node; + const promiseHandlerNames = ['then', 'catch']; + + return ( + node.type === 'ArrowFunctionExpression' + && parent.type === 'CallExpression' + && parent.callee.type === 'MemberExpression' + && !(parent.callee.object.name || "").endsWith('Dispatcher') + && promiseHandlerNames.includes(parent.callee.property.name) + ); + }, +}; + +const getNodeDescription = node => { + if (types.isPromiseHandlerArrowFunction(node)) return 'promise handler arrow function'; + if (types.isExportedArrowFunction(node)) return 'exported arrow function'; + if (types.isExportedClass(node)) return 'exported class'; + + throw new Error("Unexpected node; could not provide description", node) +}; + + +const NAMESPACEABLE_NODE = [ + types.ExportedClass, + types.PromiseHandlerArrowFunction, + types.ExportedArrowFunction, +].join(','); + +const getProperParentNode = (node) => { + if (types.isExportedClass(node)) { + return node.parent; + } + if (types.isExportedArrowFunction(node)) { + return node.parent.parent.parent; + } + if (types.isPromiseHandlerArrowFunction(node)) { + return node; + } + + return {}; +}; + +const getNamespaceCommentForNode = (node, sourceCode) => { + const parent = getProperParentNode(node); + return getLeadingCommentsForNode(parent, sourceCode) + .find( + comment => comment.value.includes('@namespace'), + ); +}; + +const collectFunctionNamespace = (node, stack) => { + if (node.type === 'CallExpression') { + if (node.callee.type === 'MemberExpression') { + stack.push(node.callee.property.name); + collectFunctionNamespace(node.callee.object, stack); + } else if (node.callee.type === 'Identifier') { + stack.push(node.callee.name); + } + } +}; + +const getNodeNamespace = (node) => { + const stack = []; + + if (node.parent.type === 'VariableDeclarator') { + stack.push(node.parent.id.name) + } else if (node.type === 'ClassDeclaration') { + stack.push(node.id.name); + } else { + collectFunctionNamespace(node.parent, stack); + } + + // not using path.sep on purpose + return stack.reverse().join('/'); +}; + +const prepareFilePath = (pathname) => { + const { + name: filename, + dir, + } = path.parse(pathname); + + const [name, postfix = ''] = filename.split('.'); + + /** + * We do not want the \\ paths on Windows, rather / => + * split and then join with correct delimiter + **/ + return path.join( + dir, + // If dir name === file name without postfix => do not repeat it + new RegExp(`${ path.sep }${ name }$`).test(dir) ? '' : name, + postfix, + ).split(path.sep) + .filter(x => x.length > 0); +}; + +const formatPackageName = (packageName) => { + const [org = '', name = ''] = packageName.split('/'); + + if (!name) { + // if there is no name => there is not ORG + if (packageName === '<%= name %>') { + return 'placeholder' + } + + return packageName; + } + + if (org === '@scandipwa') { + // Legacy support + if (name === 'scandipwa') { + return ''; + } + + return name; + } + + return `${ org.slice(1) }/${ name }`; +}; + +const getPackageName = (context) => { + const filePath = context.getFilename(); + + // if we are in a unit test, mock the package name + if (filePath === "") { + return 'TestPackage'; + } + + const modulePath = walkDirectoryUp(filePath).pathname; + const { name } = getPackageJson(modulePath); + + return name; +}; + +const getPackageRelativePath = (context) => { + const filePath = context.getFilename(); + + // if we are in a unit test, mock the relative path + if (filePath === "") { + return 'test/path'; + } + + const modulePath = walkDirectoryUp(filePath).pathname; + return path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); +}; + +const getExpectedNamespace = (node, context) => { + const packageName = getPackageName(context); + const fileRelative = getPackageRelativePath(context); + + // Not using path.join to support windows + const pathname = [ + formatPackageName(packageName), + // Trim post-fixes if they are not present + ...prepareFilePath(fileRelative), + ].join('/').replace( + // Convert to pascal-case, and trim "-" + /\b[a-z](?=[a-z]{2})/g, + (letter) => letter.toUpperCase(), + ).split('-').join(''); + + // Do not transform code to uppercase / lowercase it should be written alright + return `${ pathname }/${ getNodeNamespace(node) }`; +}; + +const getActualNamespace = ({ value: comment = '' }) => { + const { + groups: { + namespace, + } = {}, + } = comment.match(/@namespace +(?[^ ]+)/) || {}; + + return namespace; +}; + +module.exports = { + getNodeDescription, + NAMESPACEABLE_NODE, + getProperParentNode, + getNamespaceCommentForNode, + getNodeNamespace, + getExpectedNamespace, + getActualNamespace, +}; From f45478e05ad8cc10890acef09691c9f3b1fe2b27 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 26 May 2021 10:54:09 +0300 Subject: [PATCH 12/18] Improve code readability --- .../lib/util/fix-namespace-lack.js | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js index 02e77bd1..5f521623 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js @@ -1,5 +1,19 @@ const getLeadingCommentsForNode = require('./get-leading-comments'); + +const isEslintNextLineComment = comment => comment.value.includes('eslint-disable-next-line'); + +/** + * Returns true for block comments that aren't eslint-disable comments, and don't include a @license tag + */ +const isLicenseComment = (comment) => comment.value.includes('@license'); + +const createNamespaceComment = (namespace) => `/** @namespace ${ namespace } */\n`; + +const isInParentheses = (node, context) => { + return context.getSourceCode().text[node.loc.start - 1] === '('; +} + /** * Insert namespace decorator comment before the appropriate node * @param {Fixer} fixer @@ -9,35 +23,30 @@ const getLeadingCommentsForNode = require('./get-leading-comments'); */ module.exports = (fixer, node, context, namespace) => { const sourceCode = context.getSourceCode(); - const leadingComments = getLeadingCommentsForNode(node, sourceCode); + const leadingComments = getLeadingCommentsForNode(node, sourceCode).reverse(); - if (leadingComments.find(comment => comment.value.includes('@namespace'))) { - throw new Error("Could not add namespace: node already has a namespace comment") - } - - const blockComment = leadingComments.reverse().find( - comment => comment.type === 'Block' && !['eslint-disable-next-line', '@license'].some(cond => comment.value.includes(cond)) - ); - - const eslintComment = leadingComments.find( - comment => comment.value.includes('eslint-disable-next-line') - ); + const regularBlockComment = leadingComments.find(comment => { + if (comment.type !== 'Block') return false; + return !isLicenseComment(comment) && !isEslintNextLineComment(comment) + }); - if (blockComment) { + if (regularBlockComment) { return fixer.replaceText( - blockComment, - '/*' + blockComment.value.concat(`* @namespace ${namespace}`) + '\n */' + regularBlockComment, + '/*' + regularBlockComment.value.concat(`* @namespace ${ namespace }`) + '\n */', ); } + const eslintComment = leadingComments.find(isEslintNextLineComment); + + const comment = createNamespaceComment(namespace); + if (eslintComment) { - return fixer.insertTextBefore(eslintComment, `/** @namespace ${namespace} */\n`); + return fixer.insertTextBefore(eslintComment, comment); } return fixer.insertTextBefore( node, - `${ - context.getSourceCode().text[node.loc.start - 1] === '(' ? '\n' : '' - }/** @namespace ${namespace} */\n` + `${ isInParentheses(node, context) ? '\n' : '' }${ comment }`, ); -} +}; From 14c898f38d67df1cfbc696d6ef5fe03eaa6cda82 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Thu, 27 May 2021 09:25:54 +0300 Subject: [PATCH 13/18] Remove outdated rule docs (rules no longer exist) --- .../rules/no-non-extensible-components.md | 39 ------------------- .../docs/rules/use-extensible-base.md | 25 ------------ .../docs/rules/use-middleware.md | 21 ---------- 3 files changed, 85 deletions(-) delete mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/no-non-extensible-components.md delete mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-extensible-base.md delete mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-middleware.md diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/no-non-extensible-components.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/no-non-extensible-components.md deleted file mode 100644 index 40f27487..00000000 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/no-non-extensible-components.md +++ /dev/null @@ -1,39 +0,0 @@ -# Non-extensible components are not allowed. (no-non-extensible-components) - -Rules preventing the extensibility issues: - -- Non-extensible component import are not allowed. Use extensible bases instead of regular `Component` or `PureComponent`. - -- Variables and classes if declared on root level must be exported (and not by default!) - -## Rule Details - -Make sure to export the class and variable declarations - -Examples of **incorrect** code for this rule: - -```js -import { PureComponent } from 'react'; - -const B = 123; - -class A { - /** ... */ -} - -export default A; -``` - -Examples of **correct** code for this rule: - -```js -// notice no PureComponent import - -export const B = 123; - -export class A { - /** ... */ -} - -export default A; -``` diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-extensible-base.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-extensible-base.md deleted file mode 100644 index 09d9b257..00000000 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-extensible-base.md +++ /dev/null @@ -1,25 +0,0 @@ -# All components should be extensible. (use-extensible-base) - -For class to be extensible it should be derived from extensible base. Replacements of non-extensible bases are as follows and should not be imported - these are global. - -- PureComponent -> ExtensiblePureComponent -- Component -> ExtensibleComponent -- no base -> ExtensibleClass - -## Rule Details - -The `ExtensiblePureComponent`, `ExtensibleComponent` and `ExtensibleClass` requires no import. - -Examples of **incorrect** code for this rule: - -```js -import { PureComponent } from 'react'; -class A extends PureComponent { /** ... */ } -``` - -Examples of **correct** code for this rule: - -```js -// notice, no import, it is a global variable -class A extends ExtensiblePureComponent { /** ... */ } -``` diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-middleware.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-middleware.md deleted file mode 100644 index 125ec88c..00000000 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-middleware.md +++ /dev/null @@ -1,21 +0,0 @@ -# Wrap default export classes in middleware function (use-middleware) - -Wrap default export classes in middleware function in order to make classes extensible and assign namespaces to them. - -## Rule Details - -Examples of **incorrect** code for this rule: - -```js -class A { /** ... */ } - -export default A; -``` - -Examples of **correct** code for this rule: - -```js -class A { /** ... */ } - -export default middleware(A, 'My/NameSpace/A'); -``` From 0b1947740991ab2fc2aa550268a505ed3fec3fa8 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Thu, 27 May 2021 09:34:36 +0300 Subject: [PATCH 14/18] Add coner-case tests for use-namespace --- .../tests/lib/rules/use-namespace.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js index e4f0da88..a3850c0f 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js @@ -50,5 +50,27 @@ ruleTester.run("use-namespace", rule, { "export const mapStateToProps = () => {};", errors: 1, }, + { // test that eslint-disable comment treated correctly + code: "// eslint-disable-next-line whatever\n" + + "export const mapStateToProps = () => {};", + output: "/** @namespace TestPackage/Test/Path/mapStateToProps */\n" + + "// eslint-disable-next-line whatever\n" + + "export const mapStateToProps = () => {};", + errors: 2, // disabling non-existent rule "whatever" causes error + }, + { // test that license comments are ignored + code: "/** @license MIT */\n" + + "export const mapStateToProps = () => {};", + output: "/** @license MIT */\n" + + "/** @namespace TestPackage/Test/Path/mapStateToProps */\n" + + "export const mapStateToProps = () => {};", + errors: 1, + }, + { + code: "export const mapStateToProps = () => {};", + output: "/** @namespace TestPackage/Test/Path/mapStateToProps */\n" + + "export const mapStateToProps = () => {};", + errors: 1, + }, ], }); From 994981893cccbb66f8d82d4cf508be634f48f5d0 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Thu, 27 May 2021 09:44:12 +0300 Subject: [PATCH 15/18] Create doc page for use-namespace --- .../docs/rules/export-level-one.md | 2 +- .../docs/rules/use-namespace.md | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-namespace.md diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md index 8b16a72a..9fb0274a 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md @@ -1,4 +1,4 @@ -# export-level-one +# Declarations must be exported (export-level-one) In Scandi, all top-level declaration need to be [exported](https://javascript.info/import-export). This ensures that your code remains extensible. This rule applies to all top-level class, function and constant declarations. diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-namespace.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-namespace.md new file mode 100644 index 00000000..02b4d8cb --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/use-namespace.md @@ -0,0 +1,28 @@ +# Plugin endpoints must have a namespace (use-namespace) + +Exported functions and classes (as well as some other values) need to be available for extensions to plug into. +To achieve this, they must be decorated by the `@namespace` comment, and the namespace should be correct. + +Examples of **incorrect** code for this rule: + +```js +// no namespace: +export class HeaderComponent {} +``` + +```js +/** @namespace The/Wrong/Namespace/HeaderComponent */ +export class HeaderComponent {} +``` + +Examples of **correct** code for this rule: + +```js +// in TestPackage, in the file Test/Path: +/** @namespace TestPackage/Test/Path/HeaderComponent */ +export class HeaderComponent {} +``` + +## Why? +Decorating these classes and functions with namepaces ensures that your theme will work correctly with Scandi + [extensions](https://docs.scandipwa.com/developing-with-scandi/extensions). From e714817fabceb1faf4ceb4d841c05f8dbd9c52c2 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Thu, 27 May 2021 10:02:20 +0300 Subject: [PATCH 16/18] Add derived-class-names doc URL Refactor: move message to rule script --- .../lib/rules/derived-class-names.js | 15 ++++++++++++++- .../lib/util/derived-class-name.js | 10 ---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/derived-class-names.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/derived-class-names.js index eebb3246..4c44c75c 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/derived-class-names.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/derived-class-names.js @@ -2,16 +2,29 @@ * @fileoverview Class name must match the name of the file it is declared in. * @author Jegors Batovs */ +const { constructMessage } = require('../util/messages.js'); const { getIdentifierOccurrences } = require('../util/ast.js'); -const { getExpectedClassNameFromFilename, shouldClassNameBeEnforced, getUnexpectedNameMessage } = require('../util/derived-class-name.js'); +const { getExpectedClassNameFromFilename, shouldClassNameBeEnforced } = require('../util/derived-class-name.js'); const { getFilenameFromPath } = require("../util/path.js"); +const DOCUMENTATION_LINK = + "https://github.com/scandipwa/eslint/blob/master/docs/rules/derived-class-names.md"; + + +function getUnexpectedNameMessage(filename, expectedName, actualName) { + const error = `In Scandi, class names need to be based on the file name. Since the filename is ${ filename } the class name should be ${ expectedName }.`; + const help = `To fix this error, rename ${ actualName } to ${ expectedName }.`; + + return constructMessage(error, help, DOCUMENTATION_LINK); +} + module.exports = { meta: { docs: { description: 'Class name must match the name of the file it is declared in.', category: 'Coding standard', recommended: true, + url: DOCUMENTATION_LINK }, fixable: 'code', }, diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/derived-class-name.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/derived-class-name.js index 33164982..8c31e050 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/derived-class-name.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/derived-class-name.js @@ -23,17 +23,7 @@ function getExpectedClassNameFromFilename(filename) { return withCapitalizedInitial(baseName); } -function getUnexpectedNameMessage(filename, expectedName, actualName) { - const error = `In Scandi, class names need to be based on the file name. Since the filename is ${ filename } the class name should be ${ expectedName }.`; - const help = `To fix this error, rename ${ actualName } to ${ expectedName }.`; - const documentationLink = - "https://github.com/scandipwa/eslint/blob/master/docs/rules/derived-class-names.md"; - - return constructMessage(error, help, documentationLink); -} - module.exports = { shouldClassNameBeEnforced, getExpectedClassNameFromFilename, - getUnexpectedNameMessage, }; From 0b0ab96e8335f2cf27395d37ef848dcd3bdee249 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Thu, 27 May 2021 10:02:31 +0300 Subject: [PATCH 17/18] Improve use-namespace error messages --- .../lib/rules/use-namespace.js | 32 ++++++++++++++++--- .../lib/util/fix-namespace-lack.js | 7 +++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js index 82207379..d35d952d 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/use-namespace.js @@ -3,6 +3,7 @@ * @author Jegors Batovs */ +const { constructMessage } = require('../util/messages.js'); const { getNodeDescription, getActualNamespace, @@ -11,7 +12,29 @@ const { getProperParentNode, NAMESPACEABLE_NODE, } = require('../util/namespace.js'); -const fixNamespaceLack = require('../util/fix-namespace-lack.js'); +const { + insertNamespaceFix, + createNamespaceComment, +} = require('../util/fix-namespace-lack.js'); + +const DOCUMENTATION_LINK = + "https://github.com/scandipwa/eslint/blob/master/docs/rules/use-namespace.js"; + + +function getWrongNamespaceMessage(itemIdentifier, expectedNamespace, actualNamespace) { + const error = `The namespace for this ${itemIdentifier} is incorrect.`; + const help = `To fix this error, change the namespace from ${ actualNamespace } to ${ expectedNamespace }.`; + + return constructMessage(error, help, DOCUMENTATION_LINK); +} + +function getMissingNamespaceMessage(itemIdentifier, expectedNamespace) { + const error = `This ${itemIdentifier} should have a namespace specified but does not.`; + const expectedComment = createNamespaceComment(expectedNamespace); + const help = `To fix this error, add a namespace: ${ expectedComment }`; + + return constructMessage(error, help, DOCUMENTATION_LINK); +} module.exports = { meta: { @@ -19,6 +42,7 @@ module.exports = { description: 'Use @namespace comment-decorators', category: 'Extensibility', recommended: true, + url: DOCUMENTATION_LINK, }, fixable: 'code', }, @@ -34,8 +58,8 @@ module.exports = { if (!namespaceCommentString) { context.report({ node, - message: `Provide namespace for ${ getNodeDescription(node) } by using @namespace magic comment`, - fix: fixer => fixNamespaceLack( + message: getMissingNamespaceMessage(getNodeDescription(node), expectedNamespace), + fix: fixer => insertNamespaceFix( fixer, getProperParentNode(node), context, @@ -45,7 +69,7 @@ module.exports = { } else if (expectedNamespace !== namespaceCommentString) { context.report({ node, - message: `Namespace for this node is not valid! Consider changing it to ${ expectedNamespace }`, + message: getWrongNamespaceMessage(getNodeDescription(node), expectedNamespace, actualNamespace), fix: fixer => { const newNamespaceCommentContent = namespaceComment.value.replace(actualNamespace, expectedNamespace); const newNamespaceComment = namespaceComment.type === 'Block' diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js index 5f521623..edc2affb 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/lib/util/fix-namespace-lack.js @@ -21,7 +21,7 @@ const isInParentheses = (node, context) => { * @param {Context} context * @param {string} namespace */ -module.exports = (fixer, node, context, namespace) => { +const insertNamespaceFix = (fixer, node, context, namespace) => { const sourceCode = context.getSourceCode(); const leadingComments = getLeadingCommentsForNode(node, sourceCode).reverse(); @@ -50,3 +50,8 @@ module.exports = (fixer, node, context, namespace) => { `${ isInParentheses(node, context) ? '\n' : '' }${ comment }`, ); }; + +module.exports = { + insertNamespaceFix, + createNamespaceComment, +} From 303db0e97efebe4c7157fe74f9a24dc658704cc5 Mon Sep 17 00:00:00 2001 From: Reinis Mazeiks Date: Wed, 2 Jun 2021 10:34:23 +0300 Subject: [PATCH 18/18] Update docs; add additional test --- .../docs/rules/derived-class-names.md | 10 ++++++++-- .../tests/lib/rules/derived-class-names.js | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md index 632cd958..32e8d5f1 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/derived-class-names.md @@ -7,10 +7,13 @@ Examples of **incorrect** code for this rule: ```js // in Goodbye.component.js -class HelloComponent {} +class HelloComponent {} // should be GoodbyeComponent // in Footer.container.js -class FooterComponent {} +class FooterComponent {} // should be FooterContainer + +// in Hello.component.js +class Hello {} // should be HelloComponent ``` Examples of **correct** code for this rule: @@ -25,3 +28,6 @@ class FooterContainer {} ## Why? Naming classes according to the filename helps keep class names consistent and predictable across the codebase. + +## Note +Currently, some code in the Scandi core codebase does not follow this rule. However, all new code should adhere to it. diff --git a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/derived-class-names.js b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/derived-class-names.js index ebe20560..6ca0ef47 100644 --- a/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/derived-class-names.js +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/derived-class-names.js @@ -33,6 +33,12 @@ ruleTester.run("derived-class-names", rule, { output: "class GoodbyeComponent {}", errors: 1, }, + { + code: "class Hello {}", + filename: "Hello.component.js", + output: "class HelloComponent {}", + errors: 1, + }, { code: "export class FooterComponent {}", filename: "Footer.container.js",