Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New group types #1240

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions docs/rules/order.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,28 @@ 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'<String>, pattern: //<RegExp> }```, ```{ name: 'private'<String>, pattern: '^my-awesome-project/libs'<RegExp> }```

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
['sibling', 'parent'], // Then sibling and parent types. They can be mingled together
'index', // Then the index file
[{ 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', '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", "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'<String>, pattern: // <RegExp> }```.
Custom pattern behavior can be applied only for `"absolute"` and `"private"` types

You can set the options like this:

Expand Down
21 changes: 17 additions & 4 deletions src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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')],
Expand All @@ -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)
}
57 changes: 47 additions & 10 deletions src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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})
}
Expand All @@ -258,7 +259,16 @@ 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 }
Expand All @@ -268,16 +278,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
}, {})

Expand All @@ -287,6 +320,7 @@ function convertGroupsToRanks(groups) {

return omittedTypes.reduce(function(res, type) {
res[type] = groups.length

return res
}, rankObject)
}
Expand Down Expand Up @@ -390,11 +424,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 {
Expand All @@ -403,6 +439,7 @@ module.exports = {
},
}
}

let imported = []
let level = 0

Expand All @@ -417,15 +454,15 @@ 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) {
if (level !== 0 || !isStaticRequire(node) || !isInVariableDeclarator(node.parent)) {
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)
Expand Down
66 changes: 35 additions & 31 deletions tests/src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('../');
Expand All @@ -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({
Expand All @@ -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('./');
Expand All @@ -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');
Expand All @@ -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: `
Expand Down Expand Up @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -378,7 +382,7 @@ ruleTester.run('order', rule, {
}
}
`,
options: [{ 'newlines-between': 'always' }]
options: [{ 'newlines-between': 'always' }],
}),
// Option: newlines-between: 'always-and-inside-groups'
test({
Expand Down Expand Up @@ -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');
Expand Down