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..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 @@ -1,27 +1,33 @@ # 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 {} // should be GoodbyeComponent + +// in Footer.container.js +class FooterComponent {} // should be FooterContainer // in Hello.component.js -class HelloComponent { /** ... */ } +class Hello {} // should be HelloComponent ``` 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. + +## 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/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..9fb0274a --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/docs/rules/export-level-one.md @@ -0,0 +1,28 @@ +# 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. + +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(){} +``` + +# 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. 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'); -``` 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). 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/rules/export-level-one.js b/build-packages/eslint-plugin-scandipwa-guidelines/lib/rules/export-level-one.js index ebde26b4..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 @@ -3,29 +3,56 @@ * @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 DOCUMENTATION_URL = 'https://github.com/scandipwa/eslint/blob/master/docs/rules/export-level-one.md'; -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 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.`; + + return constructMessage(error, help, DOCUMENTATION_URL); }; module.exports = { @@ -35,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', }, @@ -44,17 +72,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 '), + }); }); }, }), 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..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,248 +3,86 @@ * @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 { constructMessage } = require('../util/messages.js'); +const { + getNodeDescription, + getActualNamespace, + getExpectedNamespace, + getNamespaceCommentForNode, + getProperParentNode, NAMESPACEABLE_NODE, +} = require('../util/namespace.js'); -const types = { - ExportedClass: [ - 'ExportNamedDeclaration', - 'ClassDeclaration' - ].join(' > '), +const { + insertNamespaceFix, + createNamespaceComment, +} = require('../util/fix-namespace-lack.js'); - ExportedArrowFunction: [ - 'ExportNamedDeclaration', - 'VariableDeclaration', - 'VariableDeclarator', - 'ArrowFunctionExpression' - ].join(' > '), +const DOCUMENTATION_LINK = + "https://github.com/scandipwa/eslint/blob/master/docs/rules/use-namespace.js"; - 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', +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 }.`; - 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) - ); - }, - - isHandleableArrowFunction: node => types.isExportedArrowFunction(node) - || types.isPromiseHandlerArrowFunction(node), - - detectType: 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'; - } -}; - -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 getNamespaceFromComments = (comments = []) => comments.find( - comment => comment.value.includes('@namespace') - ); - - return getNamespaceFromComments( - getLeadingCommentsForNode(getProperParentNode(node), sourceCode) - ); -}; - -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); - } - } + return constructMessage(error, help, DOCUMENTATION_LINK); } -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('.'); +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 }`; - /** - * 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 out empty strings if they exist - .filter(x => !!x); + return constructMessage(error, help, DOCUMENTATION_LINK); } -const preparePackageName = (packageName) => { - // This is on purpose not a path.sep (windows support) - 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 generateNamespace = (node, context) => { - const filePath = context.getFilename(); - const modulePath = walkDirectoryUp(filePath).pathname; - const fileRelative = path.relative(modulePath, filePath).replace(/^(\.\/)?src\//, ''); - const { name: packageName } = getPackageJson(modulePath); - - // Not using path.join to support windows - const pathname = [ - // remove @ from organization, support @scandipwa legacy namespaces - preparePackageName(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 extractNamespaceFromComment = ({ value: comment = '' }) => { - const { - groups: { - namespace - } = {} - } = comment.match(/@namespace +(?[^ ]+)/) || {}; - - return namespace; -}; - module.exports = { meta: { docs: { description: 'Use @namespace comment-decorators', category: 'Extensibility', - recommended: true + recommended: true, + url: DOCUMENTATION_LINK, }, - 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`, - fix: fixer => fixNamespaceLack( - fixer, - getProperParentNode(node), - context, - generatedNamespace - ) || [] + message: getMissingNamespaceMessage(getNodeDescription(node), expectedNamespace), + fix: fixer => insertNamespaceFix( + 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: getWrongNamespaceMessage(getNodeDescription(node), expectedNamespace, actualNamespace), 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/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, }; 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..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 @@ -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 @@ -7,37 +21,37 @@ const getLeadingCommentsForNode = require('./get-leading-comments'); * @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); + const leadingComments = getLeadingCommentsForNode(node, sourceCode).reverse(); - if (leadingComments.find(comment => comment.value.includes('@namespace'))) { - return null; - } - - 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.start - 1] === '(' ? '\n' : '' - }/** @namespace ${namespace} */\n` + `${ isInParentheses(node, context) ? '\n' : '' }${ comment }`, ); +}; + +module.exports = { + insertNamespaceFix, + createNamespaceComment, } 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, +}; 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", 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..a61e9977 --- /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: 1, + }, + { + code: "const FETCH_COUNT = 5;", + output: "export const FETCH_COUNT = 5;", + errors: 1, + }, + { + code: "function formatInput(){}", + output: "export function formatInput(){}", + errors: 1, + }, + ], +}); 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..a3850c0f --- /dev/null +++ b/build-packages/eslint-plugin-scandipwa-guidelines/tests/lib/rules/use-namespace.js @@ -0,0 +1,76 @@ +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, + }, + { + 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, + }, + { // 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, + }, + ], +}); 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"