JS-328 Improve rule S4328 (no-implicit-dependencies) (#4809)

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
 ],

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"
+  ]
 }

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';

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<typeof schema>)[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<string>,
+  dependencies: Set<string | Minimatch>,
+  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;
-}

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: [],
-  },
-);

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<string, Set<string>> = new Map();
 /**
  * Cache for the available dependencies by dirname.
  */
-const cache: Map<string, Set<string>> = new Map();
+const cache: Map<string, Set<string | Minimatch>> = new Map();
 
 let PackageJsonsByBaseDir: Record<string, File<PackageJson>[]>;
 
@@ -84,7 +85,7 @@ export function getDependencies(fileName: string) {
     return cached;
   }
 
-  const result = new Set<string>();
+  const result = new Set<string | Minimatch>();
   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<string>, dependencies: any) {
-  Object.keys(dependencies).forEach(name =>
-    result.add(name.startsWith(DefinitelyTyped) ? name.substring(DefinitelyTyped.length) : name),
-  );
+function addDependencies(
+  result: Set<string | Minimatch>,
+  dependencies: PackageJson.Dependency,
+  isGlob = false,
+) {
+  Object.keys(dependencies).forEach(name => addDependency(result, name, isGlob));
+}
+
+function addDependenciesArray(
+  result: Set<string | Minimatch>,
+  dependencies: string[],
+  isGlob = true,
+) {
+  dependencies.forEach(name => addDependency(result, name, isGlob));
+}
+
+function addDependency(result: Set<string | Minimatch>, 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,
+    );
+  }
 }

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",