diff --git a/its/ruling/src/test/expected/jsts/moose/typescript-S4328.json b/its/ruling/src/test/expected/jsts/moose/typescript-S4328.json index d3ba7c87c79..bf70f4e692b 100644 --- a/its/ruling/src/test/expected/jsts/moose/typescript-S4328.json +++ b/its/ruling/src/test/expected/jsts/moose/typescript-S4328.json @@ -11,6 +11,9 @@ 14, 16 ], +"moose:renderer/pages/_app.tsx": [ +1 +], "moose:renderer/utils/parseFileInfo.ts": [ 1 ], diff --git a/packages/jsts/src/rules/S4328/fixtures/package-json-project/package.json b/packages/jsts/src/rules/S4328/fixtures/package-json-project/package.json index bfb6eb31e07..6cfee22cf3a 100644 --- a/packages/jsts/src/rules/S4328/fixtures/package-json-project/package.json +++ b/packages/jsts/src/rules/S4328/fixtures/package-json-project/package.json @@ -9,5 +9,15 @@ }, "peerDependencies": { "peerDependency": "latest" - } + }, + "optionalDependencies": { + "optionalDependency": "latest" + }, + "_moduleAliases": { + "moduleAlias": "./module1" + }, + "workspaces": [ + "project1", + "project2" + ] } diff --git a/packages/jsts/src/rules/S4328/fixtures/ts-project-with-catch-all-path-alias/file.ts b/packages/jsts/src/rules/S4328/fixtures/ts-project-with-catch-all-path-alias/file.ts index 36db4933d90..2320a9979fb 100644 --- a/packages/jsts/src/rules/S4328/fixtures/ts-project-with-catch-all-path-alias/file.ts +++ b/packages/jsts/src/rules/S4328/fixtures/ts-project-with-catch-all-path-alias/file.ts @@ -1 +1,2 @@ // intentionally left empty +import { f } from '$b/c/d.e'; diff --git a/packages/jsts/src/rules/S4328/rule.ts b/packages/jsts/src/rules/S4328/rule.ts index 123e01ff163..d74ff9fcaa6 100644 --- a/packages/jsts/src/rules/S4328/rule.ts +++ b/packages/jsts/src/rules/S4328/rule.ts @@ -22,12 +22,11 @@ import { Rule } from 'eslint'; import * as estree from 'estree'; import builtins from 'builtin-modules'; -import * as path from 'path'; -import * as fs from 'fs'; import * as ts from 'typescript'; -import { generateMeta, RequiredParserServices, getDependencies } from '../helpers'; +import { generateMeta, getDependencies } from '../helpers'; import { FromSchema } from 'json-schema-to-ts'; import { meta, schema } from './meta'; +import { Minimatch } from 'minimatch'; const messages = { removeOrAddDependency: 'Either remove this import or add it as a dependency.', @@ -38,13 +37,11 @@ export const rule: Rule.RuleModule = { create(context: Rule.RuleContext) { const whitelist = (context.options as FromSchema)[0]?.whitelist || []; const dependencies = getDependencies(context.filename); - const aliasedPathsMappingPatterns = extractPathMappingPatterns( - context.sourceCode.parserServices, - ); - const baseUrl = getBaseUrl(context.sourceCode.parserServices); - if (aliasedPathsMappingPatterns === 'matchAll') { - // deactivates this rule altogether. - return {}; + const program = context.sourceCode.parserServices?.program; + let options: ts.CompilerOptions, host: ts.ModuleResolutionHost; + if (program) { + options = program?.getCompilerOptions(); + host = ts.createCompilerHost(options); } return { CallExpression: (node: estree.Node) => { @@ -61,9 +58,10 @@ export const rule: Rule.RuleModule = { argument, requireToken.loc!, dependencies, + context.filename, + host, + options, whitelist, - aliasedPathsMappingPatterns, - baseUrl, context, ); } @@ -76,9 +74,10 @@ export const rule: Rule.RuleModule = { module, importToken!.loc, dependencies, + context.filename, + host, + options, whitelist, - aliasedPathsMappingPatterns, - baseUrl, context, ); }, @@ -89,10 +88,11 @@ export const rule: Rule.RuleModule = { function raiseOnImplicitImport( module: estree.Literal, loc: estree.SourceLocation, - dependencies: Set, + dependencies: Set, + filename: string, + host: ts.ModuleResolutionHost | undefined, + options: ts.CompilerOptions | undefined, whitelist: string[], - aliasedPathsMappingPatterns: PathMappingPattern[], - baseUrl: string | undefined, context: Rule.RuleContext, ) { const moduleName = module.value; @@ -104,33 +104,42 @@ function raiseOnImplicitImport( return; } - if (aliasedPathsMappingPatterns.some(pattern => pattern.isApplicableTo(moduleName))) { + if (['node:', 'data:', 'file:'].some(prefix => moduleName.startsWith(prefix))) { return; } - if (['node:', 'data:', 'file:'].some(prefix => moduleName.startsWith(prefix))) { + const packageName = getPackageName(moduleName); + if (whitelist.includes(packageName)) { return; } - if (baseUrl) { - const underBaseUrlPath = path.join(baseUrl, moduleName); - const extensions = ['', '.ts', '.d.ts', '.tsx', '.js', '.jsx', '.vue', '.mjs']; - if (extensions.some(extension => fs.existsSync(underBaseUrlPath + extension))) { + if (builtins.includes(packageName)) { + return; + } + + for (const dependency of dependencies) { + if (typeof dependency === 'string') { + if (dependency === packageName) { + return; + } + } else if (dependency.match(moduleName)) { + //dependencies are globs for workspaces return; } } - const packageName = getPackageName(moduleName); - if ( - !whitelist.includes(packageName) && - !builtins.includes(packageName) && - !dependencies.has(packageName) - ) { - context.report({ - messageId: 'removeOrAddDependency', - loc, - }); + if (host && options) { + // check if Typescript can resolve path aliases and 'baseDir'-based import + const resolved = ts.resolveModuleName(moduleName, filename, options, host); + if (resolved?.resolvedModule && !resolved.resolvedModule.isExternalLibraryImport) { + return; + } } + + context.report({ + messageId: 'removeOrAddDependency', + loc, + }); } function getPackageName(name: string) { @@ -145,63 +154,3 @@ function getPackageName(name: string) { return `${parts[0]}/${parts[1]}`; } } - -/** - * The matching pattern part of a path mapping specified - * in `paths` in `tsconfig.json`. - */ -interface PathMappingPattern { - isApplicableTo(name: string): boolean; -} - -class PathMappingNoAsteriskPattern implements PathMappingPattern { - constructor(private readonly value: string) {} - isApplicableTo(name: string): boolean { - return name === this.value; - } -} - -class PathMappingSingleAsteriskPattern implements PathMappingPattern { - constructor( - private readonly prefix: string, - private readonly suffix: string, - ) {} - isApplicableTo(name: string): boolean { - return name.startsWith(this.prefix) && name.endsWith(this.suffix); - } -} - -const PATH_MAPPING_ASTERISK_PATTERN = /^([^*]*)\*([^*]*)$/; // matches any string with single asterisk '*' -const PATH_MAPPING_ASTERISK_PATTERN_PREFIX_IDX = 1; -const PATH_MAPPING_ASTERISK_PATTERN_SUFFIX_IDX = 2; -function extractPathMappingPatterns( - parserServices: RequiredParserServices, -): PathMappingPattern[] | 'matchAll' { - const compilerOptions = parserServices.program?.getCompilerOptions(); - const paths = compilerOptions?.paths ?? []; - const pathMappingPatterns: PathMappingPattern[] = []; - for (const p in paths) { - if (p === '*') { - return 'matchAll'; - } else { - const m = p.match(PATH_MAPPING_ASTERISK_PATTERN); - if (m) { - pathMappingPatterns.push( - new PathMappingSingleAsteriskPattern( - m[PATH_MAPPING_ASTERISK_PATTERN_PREFIX_IDX], - m[PATH_MAPPING_ASTERISK_PATTERN_SUFFIX_IDX], - ), - ); - } else if (!p.includes('*')) { - pathMappingPatterns.push(new PathMappingNoAsteriskPattern(p)); - } else { - // This case should not occur: `tsc` emits error if there is more than one asterisk - } - } - } - return pathMappingPatterns; -} - -function getBaseUrl(parserServices: RequiredParserServices): string | undefined { - return parserServices.program?.getCompilerOptions().baseUrl; -} diff --git a/packages/jsts/src/rules/S4328/unit.test.ts b/packages/jsts/src/rules/S4328/unit.test.ts index ff77afd8cde..71b2cc30c9d 100644 --- a/packages/jsts/src/rules/S4328/unit.test.ts +++ b/packages/jsts/src/rules/S4328/unit.test.ts @@ -61,6 +61,21 @@ ruleTester.run('Dependencies should be explicit', rule, { filename, options, }, + { + code: `import "optionalDependency";`, + filename, + options, + }, + { + code: `import "moduleAlias/bla";`, + filename, + options, + }, + { + code: `import "project1";`, + filename, + options, + }, { code: `import "@namespaced/dependency";`, filename, @@ -298,34 +313,3 @@ ruleTesterForBaseUrl.run('Imports based on baseUrl should be accepted', rule, { }, ], }); - -const ruleTesterForCatchAllExample = new RuleTester({ - parser: tsParserPath, - parserOptions: { - ecmaVersion: 2018, - sourceType: 'module', - tsconfigRootDir: path.join(fixtures, 'ts-project-with-catch-all-path-alias'), - project: './tsconfig.json', - }, -}); - -const filenameCatchAllExample = path.join(fixtures, 'ts-project-with-catch-all-path-alias/file.ts'); - -ruleTesterForCatchAllExample.run( - 'Do not report when a path mapping with "*"-pattern is used', - rule, - { - valid: [ - { - code: ` - import { f } from '$b/c/d.e'; - import { f } from 'concretegenerated'; - let f = require("this/might/be/generated").f; - `, - filename: filenameCatchAllExample, - options, - }, - ], - invalid: [], - }, -); diff --git a/packages/jsts/src/rules/helpers/package-json.ts b/packages/jsts/src/rules/helpers/package-json.ts index ca1d2218fb6..0b0a5dadd81 100644 --- a/packages/jsts/src/rules/helpers/package-json.ts +++ b/packages/jsts/src/rules/helpers/package-json.ts @@ -21,6 +21,7 @@ import path from 'path'; import { PackageJson } from 'type-fest'; import { searchFiles, File } from './find-files'; import { toUnixPath } from './files'; +import { Minimatch } from 'minimatch'; export const PACKAGE_JSON = 'package.json'; export const parsePackageJson = (_filename: string, contents: string | null) => @@ -36,7 +37,7 @@ const dirCache: Map> = new Map(); /** * Cache for the available dependencies by dirname. */ -const cache: Map> = new Map(); +const cache: Map> = new Map(); let PackageJsonsByBaseDir: Record[]>; @@ -84,7 +85,7 @@ export function getDependencies(fileName: string) { return cached; } - const result = new Set(); + const result = new Set(); cache.set(dirname, result); for (const packageJson of getNearestPackageJsons(fileName)) { @@ -141,11 +142,44 @@ function getDependenciesFromPackageJson(content: PackageJson) { if (content.peerDependencies !== undefined) { addDependencies(result, content.peerDependencies); } + if (content.optionalDependencies !== undefined) { + addDependencies(result, content.optionalDependencies); + } + if (content._moduleAliases !== undefined) { + addDependencies(result, content._moduleAliases as PackageJson.Dependency); + } + if (Array.isArray(content.workspaces)) { + addDependenciesArray(result, content.workspaces); + } else if (content.workspaces?.packages) { + addDependenciesArray(result, content.workspaces?.packages); + } return result; } -function addDependencies(result: Set, dependencies: any) { - Object.keys(dependencies).forEach(name => - result.add(name.startsWith(DefinitelyTyped) ? name.substring(DefinitelyTyped.length) : name), - ); +function addDependencies( + result: Set, + dependencies: PackageJson.Dependency, + isGlob = false, +) { + Object.keys(dependencies).forEach(name => addDependency(result, name, isGlob)); +} + +function addDependenciesArray( + result: Set, + dependencies: string[], + isGlob = true, +) { + dependencies.forEach(name => addDependency(result, name, isGlob)); +} + +function addDependency(result: Set, dependency: string, isGlob: boolean) { + if (isGlob) { + result.add(new Minimatch(dependency, { nocase: true, matchBase: true })); + } else { + result.add( + dependency.startsWith(DefinitelyTyped) + ? dependency.substring(DefinitelyTyped.length) + : dependency, + ); + } } diff --git a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json index 18531b327b1..d945f07e474 100644 --- a/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json +++ b/sonar-plugin/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json @@ -144,6 +144,7 @@ "S4322", "S4323", "S4325", + "S4328", "S4335", "S4423", "S4426",