From 9c5b45ce992e0299123935c846e72d6b90cc7c29 Mon Sep 17 00:00:00 2001 From: Tigran Kaginyan Date: Wed, 11 Apr 2018 18:19:26 +0300 Subject: [PATCH 1/4] Adding new group types and matching pattern for sort order --- src/core/importType.js | 21 ++++++++++--- src/rules/order.js | 48 +++++++++++++++++++++++------ tests/src/rules/order.js | 66 +++++++++++++++++++++------------------- 3 files changed, 90 insertions(+), 45 deletions(-) diff --git a/src/core/importType.js b/src/core/importType.js index 89b162aad3..7afb5430d3 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -17,8 +17,20 @@ function baseModule(name) { return pkg } -export function isAbsolute(name) { - return name.indexOf('/') === 0 +export function isAbsolute(name, settings, path, groups = []) { + const absoluteGroup = groups.find(({ name }) => name === 'absolute') + const pattern = absoluteGroup && absoluteGroup.pattern + const regexp = pattern && new RegExp(pattern) + + return name.indexOf('/') === 0 || regexp && regexp.test(name) +} + +export function isPrivate(name, settings, path, groups = []) { + const absoluteGroup = groups.find(({ name }) => name === 'private') + const pattern = absoluteGroup && absoluteGroup.pattern + const regexp = !!pattern && new RegExp(pattern) + + return !!regexp && regexp.test(name) } export function isBuiltIn(name, settings) { @@ -71,6 +83,7 @@ function isRelativeToSibling(name) { const typeTest = cond([ [isAbsolute, constant('absolute')], + [isPrivate, constant('private')], [isBuiltIn, constant('builtin')], [isExternalModule, constant('external')], [isScoped, constant('external')], @@ -81,6 +94,6 @@ const typeTest = cond([ [constant(true), constant('unknown')], ]) -export default function resolveImportType(name, context) { - return typeTest(name, context.settings, resolve(name, context)) +export default function resolveImportType(name, context, groups) { + return typeTest(name, context.settings, resolve(name, context), groups) } diff --git a/src/rules/order.js b/src/rules/order.js index f925a20eb4..931ff6b946 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -4,7 +4,7 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' -const defaultGroups = ['builtin', 'external', 'parent', 'sibling', 'index'] +const defaultGroups = ['builtin', 'external', 'private', 'absolute', 'parent', 'sibling', 'index'] // REPORTING AND FIXING @@ -241,13 +241,14 @@ function makeOutOfOrderReport(context, imported) { // DETECTING -function computeRank(context, ranks, name, type) { - return ranks[importType(name, context)] + +function computeRank(context, ranks, name, type, groups) { + return ranks[importType(name, context, groups)] + (type === 'import' ? 0 : 100) } -function registerNode(context, node, name, type, ranks, imported) { - const rank = computeRank(context, ranks, name, type) +function registerNode(context, node, name, type, ranks, imported, groups) { + const rank = computeRank(context, ranks, name, type, groups) + if (rank !== -1) { imported.push({name, rank, node}) } @@ -258,7 +259,7 @@ function isInVariableDeclarator(node) { (node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent)) } -const types = ['builtin', 'external', 'internal', 'parent', 'sibling', 'index'] +const types = ['builtin', 'external', 'private', 'internal', 'parent', 'sibling', 'index', 'absolute'] // Creates an object with type-rank pairs. // Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 } @@ -268,16 +269,39 @@ function convertGroupsToRanks(groups) { if (typeof group === 'string') { group = [group] } + + if ( + typeof group === 'object' && + group !== null && + group.constructor === Object + ) { + group = [group] + } + group.forEach(function(groupItem) { - if (types.indexOf(groupItem) === -1) { + if ( + typeof groupItem === 'object' && + groupItem !== null && + groupItem.constructor === Object && + groupItem.name + ) { + groupItem = groupItem.name + } + + const isCorrectGroup = types.find((typeItem) => typeItem === groupItem) + + if (!isCorrectGroup) { throw new Error('Incorrect configuration of the rule: Unknown type `' + JSON.stringify(groupItem) + '`') } + if (res[groupItem] !== undefined) { throw new Error('Incorrect configuration of the rule: `' + groupItem + '` is duplicated') } + res[groupItem] = index }) + return res }, {}) @@ -287,6 +311,7 @@ function convertGroupsToRanks(groups) { return omittedTypes.reduce(function(res, type) { res[type] = groups.length + return res }, rankObject) } @@ -390,11 +415,13 @@ module.exports = { create: function importOrderRule (context) { const options = context.options[0] || {} + const groups = options.groups || defaultGroups const newlinesBetweenImports = options['newlines-between'] || 'ignore' let ranks try { - ranks = convertGroupsToRanks(options.groups || defaultGroups) + ranks = convertGroupsToRanks(groups) + } catch (error) { // Malformed configuration return { @@ -403,6 +430,7 @@ module.exports = { }, } } + let imported = [] let level = 0 @@ -417,7 +445,7 @@ module.exports = { ImportDeclaration: function handleImports(node) { if (node.specifiers.length) { // Ignoring unassigned imports const name = node.source.value - registerNode(context, node, name, 'import', ranks, imported) + registerNode(context, node, name, 'import', ranks, imported, groups) } }, CallExpression: function handleRequires(node) { @@ -425,7 +453,7 @@ module.exports = { return } const name = node.arguments[0].value - registerNode(context, node, name, 'require', ranks, imported) + registerNode(context, node, name, 'require', ranks, imported, groups) }, 'Program:exit': function reportAndReset() { makeOutOfOrderReport(context, imported) diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 23487dac91..9d9cb49038 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -16,6 +16,7 @@ ruleTester.run('order', rule, { code: ` var fs = require('fs'); var async = require('async'); + var button = require('youla/button'); var relParent1 = require('../foo'); var relParent2 = require('../foo/bar'); var relParent3 = require('../'); @@ -27,11 +28,33 @@ ruleTester.run('order', rule, { code: ` import fs from 'fs'; import async, {foo1} from 'async'; + + import button from 'pattern1-lib/but5ton' + import button2 from 'pattern1-lib/4button' + + import ololol from 'absolute-pattern/ololol3'; + import ololol3 from 'absolute-pattern/ololo4l'; + import relParent1 from '../foo'; import relParent2, {foo2} from '../foo/bar'; import relParent3 from '../'; + import sibling, {foo3} from './foo'; + import index from './';`, + options: [ + { + groups: [ + ['builtin', 'external'], + { name: 'private', pattern: '^pattern1'}, + { name: 'absolute', pattern: '^absolute-pattern'}, + 'parent', + 'sibling', + 'index', + ], + 'newlines-between': 'always', + }, + ], }), // Multiple module of the same rank next to each other test({ @@ -42,7 +65,7 @@ ruleTester.run('order', rule, { var _ = require('lodash'); var async = require('async');`, }), - // Overriding order to be the reverse of the default order + // // Overriding order to be the reverse of the default order test({ code: ` var index = require('./'); @@ -55,7 +78,7 @@ ruleTester.run('order', rule, { `, options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}], }), - // Ignore dynamic requires + // // Ignore dynamic requires test({ code: ` var path = require('path'); @@ -82,25 +105,6 @@ ruleTester.run('order', rule, { require('fs'); }`, }), - // Ignore unknown/invalid cases - test({ - code: ` - var unknown1 = require('/unknown1'); - var fs = require('fs'); - var unknown2 = require('/unknown2'); - var async = require('async'); - var unknown3 = require('/unknown3'); - var foo = require('../foo'); - var unknown4 = require('/unknown4'); - var bar = require('../foo/bar'); - var unknown5 = require('/unknown5'); - var parent = require('../'); - var unknown6 = require('/unknown6'); - var foo = require('./foo'); - var unknown7 = require('/unknown7'); - var index = require('./'); - var unknown8 = require('/unknown8'); - `}), // Ignoring unassigned values by default (require) test({ code: ` @@ -279,7 +283,7 @@ ruleTester.run('order', rule, { } from 'bar'; import external from 'external' `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option newlines-between: 'always' with multiline imports #2 test({ @@ -290,7 +294,7 @@ ruleTester.run('order', rule, { import external from 'external' `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option newlines-between: 'always' with multiline imports #3 test({ @@ -301,7 +305,7 @@ ruleTester.run('order', rule, { import bar from './sibling'; `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option newlines-between: 'always' with not assigned import #1 test({ @@ -313,7 +317,7 @@ ruleTester.run('order', rule, { import _ from 'lodash'; `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option newlines-between: 'never' with not assigned import #2 test({ @@ -323,7 +327,7 @@ ruleTester.run('order', rule, { import 'something-else'; import _ from 'lodash'; `, - options: [{ 'newlines-between': 'never' }] + options: [{ 'newlines-between': 'never' }], }), // Option newlines-between: 'always' with not assigned require #1 test({ @@ -335,7 +339,7 @@ ruleTester.run('order', rule, { var _ = require('lodash'); `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option newlines-between: 'never' with not assigned require #2 test({ @@ -345,7 +349,7 @@ ruleTester.run('order', rule, { require('something-else'); var _ = require('lodash'); `, - options: [{ 'newlines-between': 'never' }] + options: [{ 'newlines-between': 'never' }], }), // Option newlines-between: 'never' should ignore nested require statement's #1 test({ @@ -362,7 +366,7 @@ ruleTester.run('order', rule, { } } `, - options: [{ 'newlines-between': 'never' }] + options: [{ 'newlines-between': 'never' }], }), // Option newlines-between: 'always' should ignore nested require statement's #2 test({ @@ -378,7 +382,7 @@ ruleTester.run('order', rule, { } } `, - options: [{ 'newlines-between': 'always' }] + options: [{ 'newlines-between': 'always' }], }), // Option: newlines-between: 'always-and-inside-groups' test({ @@ -423,7 +427,7 @@ ruleTester.run('order', rule, { message: '`fs` import should occur before import of `async`', }], }), - // fix order with spaces on the end of line + // // fix order with spaces on the end of line test({ code: ` var async = require('async'); From 7af1e154ae0b8b3fc9031eb694231a3d1e724694 Mon Sep 17 00:00:00 2001 From: Tigran Kaginyan Date: Wed, 11 Apr 2018 20:40:25 +0300 Subject: [PATCH 2/4] Removed "private" type from default types and updated docs accordingly --- docs/rules/order.md | 17 +++++++++++++++-- src/rules/order.js | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/rules/order.md b/docs/rules/order.md index 45bde6acc1..ca47a5c645 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -77,16 +77,29 @@ This rule supports the following options: ### `groups: [array]`: -How groups are defined, and the order to respect. `groups` must be an array of `string` or [`string`]. The only allowed `string`s are: `"builtin"`, `"external"`, `"internal"`, `"parent"`, `"sibling"`, `"index"`. The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example: +How groups are defined, and the order to respect. `groups` must be an array of types or [types]. + +The types can be either `"String"`: `"builtin"`, `"external"`, `"internal"`, `"parent"`, `"sibling"`, `"index"`, +or ```Object```: ```{ name: 'absolute', pattern: // }```, ```{ name: 'private', pattern: '^my-awesome-project/libs' }``` + +We intentionally made `"absolute"` type matching with optional pattern, because a lot of projects use aliases and have complicated absolute path logic. +The meaning of `"private"` type is to separate project/company level packages and libs from all `"external"` + +The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example: ```js [ 'builtin', // Built-in types are first + [{ name: 'private', pattern: '^my-awesome-project/libs' }, 'internal'] // The private types and internal (@myproject) are mixed together + { name: 'absolute', pattern: '^src' }, // Then absolute types ['sibling', 'parent'], // Then sibling and parent types. They can be mingled together 'index', // Then the index file // Then the rest: internal and external type ] ``` -The default value is `["builtin", "external", "parent", "sibling", "index"]`. +The default value is `["builtin", "external", "absolute", "parent", "sibling", "index"]`. +By default `"absolute"` type will be applied to any import which path starts with `"/"` if you want to change +that behavior you can specify absolute type with ```Object``` literal ```{ name: 'absolute', pattern: // }```. +Custom pattern behavior can be applied only for `"absolute"` and `"private"` types You can set the options like this: diff --git a/src/rules/order.js b/src/rules/order.js index 931ff6b946..7c983793c1 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -4,7 +4,7 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' -const defaultGroups = ['builtin', 'external', 'private', 'absolute', 'parent', 'sibling', 'index'] +const defaultGroups = ['builtin', 'external', 'private', 'absolute', 'sibling', 'index'] // REPORTING AND FIXING From 34bd4d46d8e89d87b0eb1a91cab4448c9d925ab6 Mon Sep 17 00:00:00 2001 From: Tigran Kaginyan Date: Mon, 16 Apr 2018 14:58:03 +0300 Subject: [PATCH 3/4] Updated docs based on new import types --- docs/rules/order.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/rules/order.md b/docs/rules/order.md index ca47a5c645..f6ad04c15a 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -91,14 +91,13 @@ The enforced order is the same as the order of each element in a group. Omitted 'builtin', // Built-in types are first [{ name: 'private', pattern: '^my-awesome-project/libs' }, 'internal'] // The private types and internal (@myproject) are mixed together { name: 'absolute', pattern: '^src' }, // Then absolute types - ['sibling', 'parent'], // Then sibling and parent types. They can be mingled together - 'index', // Then the index file + ['sibling', 'parent', 'index'], // Then sibling and parent types and index. They can be mingled together // Then the rest: internal and external type ] ``` The default value is `["builtin", "external", "absolute", "parent", "sibling", "index"]`. By default `"absolute"` type will be applied to any import which path starts with `"/"` if you want to change -that behavior you can specify absolute type with ```Object``` literal ```{ name: 'absolute', pattern: // }```. +that behavior you can specify absolute type with ```Object``` literal ```{ name: 'absolute', pattern: // }```. Custom pattern behavior can be applied only for `"absolute"` and `"private"` types You can set the options like this: From 28ba389e76aed272451f2b2058f4c17eaf1d71ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=93=D1=80?= =?UTF-8?q?=D0=B0=D1=87=D0=B5=D0=B2?= Date: Fri, 23 Nov 2018 18:28:37 +0300 Subject: [PATCH 4/4] return "private" type --- src/rules/order.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/rules/order.js b/src/rules/order.js index 7c983793c1..8cb62bc16b 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -4,7 +4,7 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' -const defaultGroups = ['builtin', 'external', 'private', 'absolute', 'sibling', 'index'] +const defaultGroups = ['builtin', 'external', 'private', 'absolute', 'parent', 'sibling', 'index'] // REPORTING AND FIXING @@ -259,7 +259,16 @@ function isInVariableDeclarator(node) { (node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent)) } -const types = ['builtin', 'external', 'private', 'internal', 'parent', 'sibling', 'index', 'absolute'] +const types = [ + 'builtin', + 'external', + 'private', + 'internal', + 'parent', + 'sibling', + 'index', + 'absolute', +] // Creates an object with type-rank pairs. // Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }