-
-
Notifications
You must be signed in to change notification settings - Fork 51
feat(extensions): support pathGroupOverrides
and fix
options
#327
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
Changes from all commits
c71d953
03c4f81
8cd5a89
ffe866c
79ab3ac
1474a62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"eslint-plugin-import-x": minor | ||
--- | ||
|
||
feat(extensions): support `pathGroupOverrides` and `fix` options |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,10 @@ | ||
import path from 'node:path' | ||
|
||
import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema' | ||
import type { RuleFixer } from '@typescript-eslint/utils/ts-eslint' | ||
import { minimatch } from 'minimatch' | ||
import type { MinimatchOptions } from 'minimatch' | ||
|
||
import type { FileExtension, RuleContext } from '../types.js' | ||
import { | ||
isBuiltIn, | ||
|
@@ -8,32 +13,53 @@ | |
createRule, | ||
moduleVisitor, | ||
resolve, | ||
parsePath, | ||
stringifyPath, | ||
} from '../utils/index.js' | ||
|
||
const modifierValues = ['always', 'ignorePackages', 'never'] as const | ||
|
||
const modifierSchema = { | ||
type: 'string' as const, | ||
type: 'string', | ||
enum: [...modifierValues], | ||
} | ||
} satisfies JSONSchema4 | ||
|
||
const modifierByFileExtensionSchema = { | ||
type: 'object' as const, | ||
type: 'object', | ||
patternProperties: { '.*': modifierSchema }, | ||
} | ||
} satisfies JSONSchema4 | ||
|
||
const properties = { | ||
type: 'object' as const, | ||
type: 'object', | ||
properties: { | ||
pattern: modifierByFileExtensionSchema, | ||
ignorePackages: { | ||
type: 'boolean' as const, | ||
type: 'boolean', | ||
}, | ||
checkTypeImports: { | ||
type: 'boolean' as const, | ||
type: 'boolean', | ||
}, | ||
pathGroupOverrides: { | ||
type: 'array', | ||
items: { | ||
type: 'object', | ||
properties: { | ||
pattern: { type: 'string' }, | ||
patternOptions: { type: 'object' }, | ||
action: { | ||
type: 'string', | ||
enum: ['enforce', 'ignore'], | ||
}, | ||
}, | ||
additionalProperties: false, | ||
required: ['pattern', 'action'], | ||
}, | ||
}, | ||
fix: { | ||
type: 'boolean', | ||
}, | ||
}, | ||
} | ||
} satisfies JSONSchema4 | ||
|
||
export type Modifier = (typeof modifierValues)[number] | ||
|
||
|
@@ -43,15 +69,27 @@ | |
ignorePackages?: boolean | ||
checkTypeImports?: boolean | ||
pattern: ModifierByFileExtension | ||
pathGroupOverrides?: PathGroupOverride[] | ||
fix?: boolean | ||
} | ||
|
||
export interface PathGroupOverride { | ||
pattern: string | ||
patternOptions?: Record<string, MinimatchOptions> | ||
action: 'enforce' | 'ignore' | ||
} | ||
|
||
export interface OptionsItemWithoutPatternProperty { | ||
ignorePackages?: boolean | ||
checkTypeImports?: boolean | ||
pathGroupOverrides?: PathGroupOverride[] | ||
fix?: boolean | ||
} | ||
|
||
export type Options = | ||
| [] | ||
| [OptionsItemWithoutPatternProperty] | ||
| [OptionsItemWithPatternProperty] | ||
| [Modifier] | ||
| [Modifier, OptionsItemWithoutPatternProperty] | ||
| [Modifier, OptionsItemWithPatternProperty] | ||
|
@@ -63,16 +101,20 @@ | |
pattern?: Record<string, Modifier> | ||
ignorePackages?: boolean | ||
checkTypeImports?: boolean | ||
pathGroupOverrides?: PathGroupOverride[] | ||
fix?: boolean | ||
} | ||
|
||
export type MessageId = 'missing' | 'missingKnown' | 'unexpected' | ||
export type MessageId = 'missing' | 'missingKnown' | 'unexpected' | 'addMissing' | ||
|
||
function buildProperties(context: RuleContext<MessageId, Options>) { | ||
const result: Required<NormalizedOptions> = { | ||
defaultConfig: 'never', | ||
pattern: {}, | ||
ignorePackages: false, | ||
checkTypeImports: false, | ||
pathGroupOverrides: [], | ||
fix: false, | ||
} | ||
|
||
for (const obj of context.options) { | ||
|
@@ -88,16 +130,16 @@ | |
|
||
// If this is not the new structure, transfer all props to result.pattern | ||
if ( | ||
(!('pattern' in obj) || obj.pattern === undefined) && | ||
obj.ignorePackages === undefined && | ||
obj.checkTypeImports === undefined | ||
(!('pattern' in obj) || obj.pattern == null) && | ||
obj.ignorePackages == null && | ||
obj.checkTypeImports == null | ||
) { | ||
Object.assign(result.pattern, obj) | ||
continue | ||
} | ||
|
||
// If pattern is provided, transfer all props | ||
if ('pattern' in obj && obj.pattern !== undefined) { | ||
if ('pattern' in obj && obj.pattern != null) { | ||
Object.assign(result.pattern, obj.pattern) | ||
} | ||
|
||
|
@@ -109,6 +151,14 @@ | |
if (typeof obj.checkTypeImports === 'boolean') { | ||
result.checkTypeImports = obj.checkTypeImports | ||
} | ||
|
||
if (obj.fix != null) { | ||
result.fix = Boolean(obj.fix) | ||
} | ||
|
||
if (Array.isArray(obj.pathGroupOverrides)) { | ||
result.pathGroupOverrides = obj.pathGroupOverrides | ||
} | ||
} | ||
|
||
if (result.defaultConfig === 'ignorePackages') { | ||
|
@@ -124,14 +174,18 @@ | |
return false | ||
} | ||
const slashCount = file.split('/').length - 1 | ||
return slashCount === 0 || (isScoped(file) && slashCount <= 1) | ||
} | ||
|
||
if (slashCount === 0) { | ||
return true | ||
} | ||
if (isScoped(file) && slashCount <= 1) { | ||
return true | ||
function computeOverrideAction( | ||
pathGroupOverrides: PathGroupOverride[], | ||
path: string, | ||
) { | ||
for (const { pattern, patternOptions, action } of pathGroupOverrides) { | ||
if (minimatch(path, pattern, patternOptions || { nocomment: true })) { | ||
return action | ||
} | ||
} | ||
return false | ||
} | ||
|
||
export default createRule<Options, MessageId>({ | ||
|
@@ -143,6 +197,8 @@ | |
description: | ||
'Ensure consistent use of file extension within the import path.', | ||
}, | ||
fixable: 'code', | ||
hasSuggestions: true, | ||
schema: { | ||
anyOf: [ | ||
{ | ||
|
@@ -178,6 +234,8 @@ | |
'Missing file extension "{{extension}}" for "{{importPath}}"', | ||
unexpected: | ||
'Unexpected use of file extension "{{extension}}" for "{{importPath}}"', | ||
addMissing: | ||
'Add "{{extension}}" file extension from "{{importPath}}" into "{{fixedImportPath}}"', | ||
}, | ||
}, | ||
defaultOptions: [], | ||
|
@@ -221,16 +279,29 @@ | |
|
||
const importPathWithQueryString = source.value | ||
|
||
// If not undefined, the user decided if rules are enforced on this import | ||
const overrideAction = computeOverrideAction( | ||
props.pathGroupOverrides || [], | ||
importPathWithQueryString, | ||
) | ||
|
||
if (overrideAction === 'ignore') { | ||
return | ||
} | ||
|
||
// don't enforce anything on builtins | ||
if (isBuiltIn(importPathWithQueryString, context.settings)) { | ||
if ( | ||
!overrideAction && | ||
isBuiltIn(importPathWithQueryString, context.settings) | ||
) { | ||
return | ||
} | ||
|
||
const importPath = importPathWithQueryString.replace(/\?(.*)$/, '') | ||
|
||
// don't enforce in root external packages as they may have names with `.js`. | ||
// Like `import Decimal from decimal.js`) | ||
if (isExternalRootModule(importPath)) { | ||
if (!overrideAction && isExternalRootModule(importPath)) { | ||
return | ||
} | ||
|
||
|
@@ -261,17 +332,55 @@ | |
} | ||
const extensionRequired = isUseOfExtensionRequired( | ||
extension, | ||
isPackage, | ||
!overrideAction && isPackage, | ||
) | ||
const extensionForbidden = isUseOfExtensionForbidden(extension) | ||
if (extensionRequired && !extensionForbidden) { | ||
const { pathname, query, hash } = parsePath( | ||
importPathWithQueryString, | ||
) | ||
const fixedImportPath = stringifyPath({ | ||
pathname: `${ | ||
/([\\/]|[\\/]?\.?\.)$/.test(pathname) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding an inline comment explaining the regex There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephenjason89 You also need to change your There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. Thanks! |
||
? `${ | ||
pathname.endsWith('/') ? pathname.slice(0, -1) : pathname | ||
}/index.${extension}` | ||
: `${pathname}.${extension}` | ||
}`, | ||
query, | ||
hash, | ||
}) | ||
const fixOrSuggest = { | ||
fix(fixer: RuleFixer) { | ||
return fixer.replaceText( | ||
source, | ||
JSON.stringify(fixedImportPath), | ||
) | ||
}, | ||
} | ||
context.report({ | ||
node: source, | ||
messageId: extension ? 'missingKnown' : 'missing', | ||
data: { | ||
extension, | ||
importPath: importPathWithQueryString, | ||
}, | ||
...(extension && | ||
(props.fix | ||
? fixOrSuggest | ||
: { | ||
suggest: [ | ||
{ | ||
...fixOrSuggest, | ||
messageId: 'addMissing', | ||
data: { | ||
extension, | ||
importPath: importPathWithQueryString, | ||
fixedImportPath: fixedImportPath, | ||
}, | ||
}, | ||
], | ||
})), | ||
}) | ||
JounQin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} else if ( | ||
|
@@ -286,6 +395,14 @@ | |
extension, | ||
importPath: importPathWithQueryString, | ||
}, | ||
...(props.fix && { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add suggestions here in another PR. |
||
fix(fixer) { | ||
return fixer.replaceText( | ||
source, | ||
JSON.stringify(importPath.slice(0, -(extension.length + 1))), | ||
) | ||
}, | ||
}), | ||
}) | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
export interface ParsedPath { | ||
pathname: string | ||
query: string | ||
hash: string | ||
} | ||
|
||
export const parsePath = (path: string): ParsedPath => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephenjason89 You'd better to use this pattern to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. |
||
const hashIndex = path.indexOf('#') | ||
const queryIndex = path.indexOf('?') | ||
const hasHash = hashIndex !== -1 | ||
const hash = hasHash ? path.slice(hashIndex) : '' | ||
const hasQuery = queryIndex !== -1 && (!hasHash || queryIndex < hashIndex) | ||
const query = hasQuery | ||
? path.slice(queryIndex, hasHash ? hashIndex : undefined) | ||
: '' | ||
const pathname = hasQuery | ||
? path.slice(0, queryIndex) | ||
: hasHash | ||
? path.slice(0, hashIndex) | ||
: path | ||
return { pathname, query, hash } | ||
} | ||
|
||
export const stringifyPath = ({ pathname, query, hash }: ParsedPath) => | ||
pathname + query + hash |
Uh oh!
There was an error while loading. Please reload this page.