Skip to content

Commit

Permalink
[eslint-plugin] Add new import-related rules to the ESLint plugin (mi…
Browse files Browse the repository at this point in the history
…crosoft#4889)

* Add new eslint rules

* Consume new eslint rules in Rushstack

* Update to newer eslint

* Fix lint issues

* Disable fixes when running in production mode

* Rush change
  • Loading branch information
D4N14L authored Aug 14, 2024
1 parent 01173c3 commit 0381a02
Show file tree
Hide file tree
Showing 39 changed files with 1,219 additions and 32 deletions.
2 changes: 1 addition & 1 deletion apps/heft/src/cli/HeftActionRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { HeftParameterManager } from '../pluginFramework/HeftParameterManager';
import { TaskOperationRunner } from '../operations/runners/TaskOperationRunner';
import { PhaseOperationRunner } from '../operations/runners/PhaseOperationRunner';
import type { HeftPhase } from '../pluginFramework/HeftPhase';
import type { IHeftAction, IHeftActionOptions } from '../cli/actions/IHeftAction';
import type { IHeftAction, IHeftActionOptions } from './actions/IHeftAction';
import type {
IHeftLifecycleCleanHookOptions,
IHeftLifecycleSession,
Expand Down
4 changes: 2 additions & 2 deletions apps/heft/src/pluginFramework/HeftTask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import type {
IHeftConfigurationJsonTaskSpecifier,
IHeftConfigurationJsonPluginSpecifier
} from '../utilities/CoreConfigFiles';
import type { IHeftTaskPlugin } from '../pluginFramework/IHeftPlugin';
import type { IScopedLogger } from '../pluginFramework/logging/ScopedLogger';
import type { IHeftTaskPlugin } from './IHeftPlugin';
import type { IScopedLogger } from './logging/ScopedLogger';

const RESERVED_TASK_NAMES: Set<string> = new Set(['clean']);

Expand Down
2 changes: 1 addition & 1 deletion apps/lockfile-explorer-web/src/store/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// See LICENSE in the project root for license information.

import { type TypedUseSelectorHook, useDispatch, useSelector } from 'react-redux';
import type { RootState, AppDispatch } from './';
import type { RootState, AppDispatch } from '.';

// Use throughout your app instead of plain `useDispatch` and `useSelector`
export const useAppDispatch: () => AppDispatch = useDispatch;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/api-extractor-model",
"comment": "",
"type": "none"
}
],
"packageName": "@microsoft/api-extractor-model"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/load-themed-styles",
"comment": "",
"type": "none"
}
],
"packageName": "@microsoft/load-themed-styles"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/loader-load-themed-styles",
"comment": "",
"type": "none"
}
],
"packageName": "@microsoft/loader-load-themed-styles"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/eslint-plugin",
"comment": "Add 4 new ESLint rules: \"@rushstack/no-backslash-imports\", used to prevent backslashes in import and require statements; \"@rushstack/no-external-local-imports\", used to prevent referencing external depedencies in import and require statements; \"@rushstack/no-transitive-dependency-imports\", used to prevent referencing transitive dependencies (ie. dependencies of dependencies) in import and require statements; and \"@rushstack/normalized-imports\", used to ensure that the most direct path to a dependency is provided in import and require statements",
"type": "minor"
}
],
"packageName": "@rushstack/eslint-plugin"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/heft-lint-plugin",
"comment": "Unintrusively disable \"--fix\" mode when running in \"--production\" mode",
"type": "patch"
}
],
"packageName": "@rushstack/heft-lint-plugin"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/heft",
"comment": "",
"type": "none"
}
],
"packageName": "@rushstack/heft"
}
119 changes: 119 additions & 0 deletions eslint/eslint-plugin/src/LintUtilities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import * as path from 'path';
import { ESLintUtils, TSESTree, type TSESLint } from '@typescript-eslint/utils';
import type { Program } from 'typescript';

export interface IParsedImportSpecifier {
loader?: string;
importTarget: string;
loaderOptions?: string;
}

// Regex to parse out the import target from the specifier. Expected formats are:
// - '<target>'
// - '<loader>!<target>'
// - '<target>?<loader-options>'
// - '<loader>!<target>?<loader-options>'
const LOADER_CAPTURE_GROUP: 'loader' = 'loader';
const IMPORT_TARGET_CAPTURE_GROUP: 'importTarget' = 'importTarget';
const LOADER_OPTIONS_CAPTURE_GROUP: 'loaderOptions' = 'loaderOptions';
// eslint-disable-next-line @rushstack/security/no-unsafe-regexp
const SPECIFIER_REGEX: RegExp = new RegExp(
`^((?<${LOADER_CAPTURE_GROUP}>(!|-!|!!).+)!)?` +
`(?<${IMPORT_TARGET_CAPTURE_GROUP}>[^!?]+)` +
`(\\?(?<${LOADER_OPTIONS_CAPTURE_GROUP}>.*))?$`
);

export function getFilePathFromContext(context: TSESLint.RuleContext<string, unknown[]>): string {
return context.physicalFilename || context.filename;
}

export function getRootDirectoryFromContext(
context: TSESLint.RuleContext<string, unknown[]>
): string | undefined {
let rootDirectory: string | undefined;
try {
// First attempt to get the root directory from the tsconfig baseUrl, then the program current directory
const program: Program | null | undefined = (
context.sourceCode?.parserServices ?? ESLintUtils.getParserServices(context)
).program;
rootDirectory = program?.getCompilerOptions().baseUrl ?? program?.getCurrentDirectory();
} catch {
// Ignore the error if we cannot retrieve a TS program
}

// Fall back to the parserOptions.tsconfigRootDir if available, otherwise the eslint working directory
if (!rootDirectory) {
rootDirectory = context.parserOptions?.tsconfigRootDir ?? context.getCwd?.();
}

return rootDirectory;
}

export function parseImportSpecifierFromExpression(
importExpression: TSESTree.Expression
): IParsedImportSpecifier | undefined {
if (
!importExpression ||
importExpression.type !== TSESTree.AST_NODE_TYPES.Literal ||
typeof importExpression.value !== 'string'
) {
// Can't determine the path of the import target, return
return undefined;
}

// Extract the target of the import, stripping out webpack loaders and query strings. The regex will
// also ensure that the import target is a relative path.
const specifierMatch: RegExpMatchArray | null = importExpression.value.match(SPECIFIER_REGEX);
if (!specifierMatch?.groups) {
// Can't determine the path of the import target, return
return undefined;
}

const loader: string | undefined = specifierMatch.groups[LOADER_CAPTURE_GROUP];
const importTarget: string = specifierMatch.groups[IMPORT_TARGET_CAPTURE_GROUP];
const loaderOptions: string | undefined = specifierMatch.groups[LOADER_OPTIONS_CAPTURE_GROUP];
return { loader, importTarget, loaderOptions };
}

export function serializeImportSpecifier(parsedImportPath: IParsedImportSpecifier): string {
const { loader, importTarget, loaderOptions } = parsedImportPath;
return `${loader ? `${loader}!` : ''}${importTarget}${loaderOptions ? `?${loaderOptions}` : ''}`;
}

export function getImportPathFromExpression(
importExpression: TSESTree.Expression,
relativeImportsOnly: boolean = true
): string | undefined {
const parsedImportSpecifier: IParsedImportSpecifier | undefined =
parseImportSpecifierFromExpression(importExpression);
if (
!parsedImportSpecifier ||
(relativeImportsOnly && !parsedImportSpecifier.importTarget.startsWith('.'))
) {
// The import target isn't a path, return
return undefined;
}
return parsedImportSpecifier?.importTarget;
}

export function getImportAbsolutePathFromExpression(
context: TSESLint.RuleContext<string, unknown[]>,
importExpression: TSESTree.Expression,
relativeImportsOnly: boolean = true
): string | undefined {
const importPath: string | undefined = getImportPathFromExpression(importExpression, relativeImportsOnly);
if (importPath === undefined) {
// Can't determine the absolute path of the import target, return
return undefined;
}

const filePath: string = getFilePathFromContext(context);
const fileDirectory: string = path.dirname(filePath);

// Combine the import path with the absolute path of the file parent directory to get the
// absolute path of the import target
return path.resolve(fileDirectory, importPath);
}
16 changes: 16 additions & 0 deletions eslint/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
import { TSESLint } from '@typescript-eslint/utils';

import { hoistJestMock } from './hoist-jest-mock';
import { noBackslashImportsRule } from './no-backslash-imports';
import { noExternalLocalImportsRule } from './no-external-local-imports';
import { noNewNullRule } from './no-new-null';
import { noNullRule } from './no-null';
import { noTransitiveDependencyImportsRule } from './no-transitive-dependency-imports';
import { noUntypedUnderscoreRule } from './no-untyped-underscore';
import { normalizedImportsRule } from './normalized-imports';
import { typedefVar } from './typedef-var';

interface IPlugin {
Expand All @@ -18,15 +22,27 @@ const plugin: IPlugin = {
// Full name: "@rushstack/hoist-jest-mock"
'hoist-jest-mock': hoistJestMock,

// Full name: "@rushstack/no-backslash-imports"
'no-backslash-imports': noBackslashImportsRule,

// Full name: "@rushstack/no-external-local-imports"
'no-external-local-imports': noExternalLocalImportsRule,

// Full name: "@rushstack/no-new-null"
'no-new-null': noNewNullRule,

// Full name: "@rushstack/no-null"
'no-null': noNullRule,

// Full name: "@rushstack/no-transitive-dependency-imports"
'no-transitive-dependency-imports': noTransitiveDependencyImportsRule,

// Full name: "@rushstack/no-untyped-underscore"
'no-untyped-underscore': noUntypedUnderscoreRule,

// Full name: "@rushstack/normalized-imports"
'normalized-imports': normalizedImportsRule,

// Full name: "@rushstack/typedef-var"
'typedef-var': typedefVar
}
Expand Down
71 changes: 71 additions & 0 deletions eslint/eslint-plugin/src/no-backslash-imports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import type { TSESTree, TSESLint } from '@typescript-eslint/utils';
import {
parseImportSpecifierFromExpression,
serializeImportSpecifier,
type IParsedImportSpecifier
} from './LintUtilities';

export const MESSAGE_ID: 'no-backslash-imports' = 'no-backslash-imports';
type RuleModule = TSESLint.RuleModule<typeof MESSAGE_ID, []>;
type RuleContext = TSESLint.RuleContext<typeof MESSAGE_ID, []>;

export const noBackslashImportsRule: RuleModule = {
defaultOptions: [],
meta: {
type: 'problem',
messages: {
[MESSAGE_ID]: 'The specified import target path contains backslashes.'
},
schema: [],
docs: {
description: 'Prevents imports using paths that use backslashes',
url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin'
},
fixable: 'code'
},
create: (context: RuleContext) => {
const checkImportExpression: (importExpression: TSESTree.Expression | null) => void = (
importExpression: TSESTree.Expression | null
) => {
if (!importExpression) {
// Can't validate, return
return;
}

// Determine the target file path and find the most direct relative path from the source file
const importSpecifier: IParsedImportSpecifier | undefined =
parseImportSpecifierFromExpression(importExpression);
if (importSpecifier === undefined) {
// Can't validate, return
return;
}

// Check if the import path contains backslashes. If it does, suggest a fix to replace them with forward
// slashes.
const { importTarget } = importSpecifier;
if (importTarget.includes('\\')) {
context.report({
node: importExpression,
messageId: MESSAGE_ID,
fix: (fixer: TSESLint.RuleFixer) => {
const normalizedSpecifier: IParsedImportSpecifier = {
...importSpecifier,
importTarget: importTarget.replace(/\\/g, '/')
};
return fixer.replaceText(importExpression, `'${serializeImportSpecifier(normalizedSpecifier)}'`);
}
});
}
};

return {
ImportDeclaration: (node: TSESTree.ImportDeclaration) => checkImportExpression(node.source),
ImportExpression: (node: TSESTree.ImportExpression) => checkImportExpression(node.source),
ExportAllDeclaration: (node: TSESTree.ExportAllDeclaration) => checkImportExpression(node.source),
ExportNamedDeclaration: (node: TSESTree.ExportNamedDeclaration) => checkImportExpression(node.source)
};
}
};
Loading

0 comments on commit 0381a02

Please sign in to comment.