diff --git a/package-lock.json b/package-lock.json index 487ad0f..3e4cc24 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@matrixai/lint", - "version": "0.4.2", + "version": "0.4.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@matrixai/lint", - "version": "0.4.2", + "version": "0.4.3", "license": "Apache-2.0", "dependencies": { "@eslint/compat": "^1.2.5", @@ -25,6 +25,7 @@ "eslint-plugin-react-hooks": "^5.1.0", "eslint-plugin-tailwindcss": "^3.18.0", "globals": "^16.2.0", + "minimatch": "^10.0.1", "prettier": "^3.0.0" }, "bin": { @@ -38,7 +39,6 @@ "jest": "^29.6.2", "jest-extended": "^4.0.2", "jest-junit": "^16.0.0", - "minimatch": "^10.0.1", "shx": "^0.3.4", "tsx": "^3.12.7", "typedoc": "^0.24.8", @@ -7956,7 +7956,6 @@ "version": "10.0.1", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.0.1.tgz", "integrity": "sha512-ethXTt3SGGR+95gudmqJ1eNhRO7eGEGIgYA9vnPatK4/etz2MEVDno5GMCibdMTuBMyElzIlgxMna3K94XDIDQ==", - "dev": true, "license": "ISC", "dependencies": { "brace-expansion": "^2.0.1" diff --git a/package.json b/package.json index a06c196..3d10f08 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@matrixai/lint", - "version": "0.4.2", + "version": "0.4.3", "author": "Roger Qiu", "description": "Org wide custom eslint rules", "license": "Apache-2.0", @@ -59,6 +59,7 @@ "eslint-plugin-react-hooks": "^5.1.0", "eslint-plugin-tailwindcss": "^3.18.0", "globals": "^16.2.0", + "minimatch": "^10.0.1", "prettier": "^3.0.0" }, "devDependencies": { @@ -69,7 +70,6 @@ "jest": "^29.6.2", "jest-extended": "^4.0.2", "jest-junit": "^16.0.0", - "minimatch": "^10.0.1", "shx": "^0.3.4", "tsx": "^3.12.7", "typedoc": "^0.24.8", diff --git a/src/domains/eslint.ts b/src/domains/eslint.ts index 5f35f40..b6bfefc 100644 --- a/src/domains/eslint.ts +++ b/src/domains/eslint.ts @@ -1,7 +1,6 @@ import type { LintDomainPlugin } from './engine.js'; import { - collectFilesByExtensions, - relativizeFiles, + resolveFilesFromPatterns, resolveSearchRootsFromPatterns, } from './files.js'; import * as utils from '../utils.js'; @@ -53,23 +52,35 @@ function createESLintDomainPlugin(): LintDomainPlugin { domain: 'eslint', description: 'Lint JavaScript/TypeScript/JSON files with ESLint.', detect: ({ eslintPatterns }) => { - const patterns = resolveESLintDetectionPatterns(eslintPatterns); - const searchRoots = resolveSearchRootsFromPatterns(patterns); - const matchedFiles = collectFilesByExtensions( - searchRoots, + if (eslintPatterns != null && eslintPatterns.length > 0) { + const searchRoots = resolveSearchRootsFromPatterns(eslintPatterns); + + return { + relevant: searchRoots.length > 0, + relevanceReason: + searchRoots.length > 0 + ? undefined + : 'No ESLint-supported files matched in effective scope.', + available: true, + availabilityKind: 'required' as const, + }; + } + + const detectionPatterns = resolveESLintDetectionPatterns(eslintPatterns); + const matchedFiles = resolveFilesFromPatterns( + detectionPatterns, ESLINT_FILE_EXTENSIONS, ); - const matchedRelativeFiles = relativizeFiles(matchedFiles); return { - relevant: matchedRelativeFiles.length > 0, + relevant: matchedFiles.length > 0, relevanceReason: - matchedRelativeFiles.length > 0 + matchedFiles.length > 0 ? undefined : 'No ESLint-supported files matched in effective scope.', available: true, availabilityKind: 'required' as const, - matchedFiles: matchedRelativeFiles, + matchedFiles, }; }, run: async ({ @@ -85,11 +96,15 @@ function createESLintDomainPlugin(): LintDomainPlugin { } try { + const explicitPatterns = + eslintPatterns != null && eslintPatterns.length > 0 + ? resolveFilesFromPatterns(eslintPatterns, ESLINT_FILE_EXTENSIONS) + : undefined; const hadLintingErrors = await utils.runESLint({ fix, logger, configPath: chosenConfig, - explicitGlobs: eslintPatterns, + explicitGlobs: explicitPatterns, }); return { hadFailure: hadLintingErrors }; diff --git a/src/domains/files.ts b/src/domains/files.ts index 814c4a9..3e33b96 100644 --- a/src/domains/files.ts +++ b/src/domains/files.ts @@ -1,22 +1,58 @@ import path from 'node:path'; import process from 'node:process'; import fs from 'node:fs'; +import { minimatch } from 'minimatch'; const GLOB_META_PATTERN = /[*?[\]{}()!+@]/; const EXCLUDED_DIR_NAMES = new Set(['.git', 'node_modules', 'dist']); +function normalizePathForGlob(value: string): string { + return value.replace(/\\/g, '/').replace(/^\.\//, ''); +} + +function normalizePatternForSearchRoot(pattern: string): string { + return pattern.trim().replace(/\\/g, '/'); +} + +function toPosixRelativePath(filePath: string, cwd = process.cwd()): string { + const relativePath = path.relative(cwd, filePath).split(path.sep).join('/'); + if (relativePath === '') { + return '.'; + } + return relativePath; +} + +function normalizePatternForMatching( + pattern: string, + cwd = process.cwd(), +): string { + const normalizedPattern = normalizePathForGlob(pattern.trim()); + if (normalizedPattern.length === 0) { + return ''; + } + + const platformPattern = normalizedPattern.split('/').join(path.sep); + const absolutePattern = path.isAbsolute(platformPattern) + ? platformPattern + : path.resolve(cwd, platformPattern); + + return toPosixRelativePath(absolutePattern, cwd); +} + function isGlobPattern(value: string): boolean { return GLOB_META_PATTERN.test(value); } function patternToSearchRoot(pattern: string, cwd = process.cwd()): string { - if (!isGlobPattern(pattern)) { - return path.resolve(cwd, pattern); + const normalizedPattern = normalizePatternForSearchRoot(pattern); + + if (!isGlobPattern(normalizedPattern)) { + return path.resolve(cwd, normalizedPattern); } - const normalizedPattern = pattern.split('/').join(path.sep); - const segments = normalizedPattern + const platformPattern = normalizedPattern.split('/').join(path.sep); + const segments = platformPattern .split(path.sep) .filter((segment) => segment.length > 0); const rootSegments: string[] = []; @@ -105,6 +141,92 @@ function collectFilesByExtensions( return [...matchedFiles].sort(); } +function resolveFilesFromPatterns( + patterns: readonly string[], + extensions: readonly string[], + cwd = process.cwd(), +): string[] { + const normalizedPatterns = [...new Set(patterns)] + .map((pattern) => pattern.trim()) + .filter((pattern) => pattern.length > 0); + + if (normalizedPatterns.length === 0) { + return []; + } + + const extensionSet = new Set( + extensions.map((extension) => extension.toLowerCase()), + ); + const matchedFiles = new Set(); + const literalFiles = new Set(); + const literalDirectories = new Set(); + const globPatterns: string[] = []; + + for (const pattern of normalizedPatterns) { + const platformPattern = pattern.replace(/\//g, path.sep); + const absolutePath = path.isAbsolute(platformPattern) + ? platformPattern + : path.resolve(cwd, platformPattern); + let stats: fs.Stats | undefined; + + try { + stats = fs.statSync(absolutePath); + } catch { + stats = undefined; + } + + if (stats?.isFile()) { + literalFiles.add(absolutePath); + continue; + } + + if (stats?.isDirectory()) { + literalDirectories.add(absolutePath); + continue; + } + + if (isGlobPattern(pattern)) { + globPatterns.push(pattern); + continue; + } + } + + for (const literalFile of literalFiles) { + const extension = path.extname(literalFile).toLowerCase(); + if (extensionSet.has(extension)) { + matchedFiles.add(literalFile); + } + } + + for (const literalDirectory of literalDirectories) { + const files = collectFilesByExtensions([literalDirectory], extensions); + files.forEach((file) => matchedFiles.add(file)); + } + + if (globPatterns.length > 0) { + const globRoots = resolveSearchRootsFromPatterns(globPatterns, cwd); + const globCandidates = collectFilesByExtensions(globRoots, extensions); + const normalizedGlobPatterns = globPatterns + .map((pattern) => normalizePatternForMatching(pattern, cwd)) + .filter((pattern) => pattern.length > 0); + + for (const candidate of globCandidates) { + const relativeCandidatePath = toPosixRelativePath(candidate, cwd); + if ( + normalizedGlobPatterns.some((pattern) => + minimatch(relativeCandidatePath, pattern, { + dot: true, + }), + ) + ) { + matchedFiles.add(candidate); + } + } + } + + return relativizeFiles([...matchedFiles].sort(), cwd); +} + function relativizeFiles( files: readonly string[], cwd = process.cwd(), @@ -121,5 +243,6 @@ function relativizeFiles( export { resolveSearchRootsFromPatterns, collectFilesByExtensions, + resolveFilesFromPatterns, relativizeFiles, }; diff --git a/src/domains/markdown.ts b/src/domains/markdown.ts index 93da3dc..e97d2ce 100644 --- a/src/domains/markdown.ts +++ b/src/domains/markdown.ts @@ -4,11 +4,7 @@ import process from 'node:process'; import childProcess from 'node:child_process'; import fs from 'node:fs'; import { createRequire } from 'node:module'; -import { - collectFilesByExtensions, - relativizeFiles, - resolveSearchRootsFromPatterns, -} from './files.js'; +import { resolveFilesFromPatterns } from './files.js'; const platform = os.platform(); const MARKDOWN_FILE_EXTENSIONS = ['.md', '.mdx'] as const; @@ -21,14 +17,11 @@ const DEFAULT_MARKDOWN_SEARCH_ROOTS = [ './docs', ]; -function collectMarkdownFilesFromScope( - searchRoots: readonly string[], -): string[] { - const matchedFiles = collectFilesByExtensions( - searchRoots, +function collectMarkdownFilesFromScope(patterns: readonly string[]): string[] { + const matchedRelativeFiles = resolveFilesFromPatterns( + patterns, MARKDOWN_FILE_EXTENSIONS, ); - const matchedRelativeFiles = relativizeFiles(matchedFiles); for (const rootFile of [...DEFAULT_MARKDOWN_ROOT_FILES].reverse()) { if (!matchedRelativeFiles.includes(rootFile) && fs.existsSync(rootFile)) { @@ -39,6 +32,14 @@ function collectMarkdownFilesFromScope( return matchedRelativeFiles; } +function resolveMarkdownPatterns( + markdownPatterns: readonly string[] | undefined, +): string[] { + return markdownPatterns != null && markdownPatterns.length > 0 + ? [...markdownPatterns] + : [...DEFAULT_MARKDOWN_SEARCH_ROOTS]; +} + function createMarkdownDomainPlugin({ prettierConfigPath, }: { @@ -48,12 +49,8 @@ function createMarkdownDomainPlugin({ domain: 'markdown', description: 'Format and check Markdown/MDX files with Prettier.', detect: ({ markdownPatterns }) => { - const searchPatterns = - markdownPatterns != null && markdownPatterns.length > 0 - ? markdownPatterns - : DEFAULT_MARKDOWN_SEARCH_ROOTS; - const searchRoots = resolveSearchRootsFromPatterns(searchPatterns); - const matchedFiles = collectMarkdownFilesFromScope(searchRoots); + const patterns = resolveMarkdownPatterns(markdownPatterns); + const matchedFiles = collectMarkdownFilesFromScope(patterns); return { relevant: matchedFiles.length > 0, diff --git a/src/domains/shell.ts b/src/domains/shell.ts index 854cfe9..298e106 100644 --- a/src/domains/shell.ts +++ b/src/domains/shell.ts @@ -2,17 +2,22 @@ import type { LintDomainPlugin } from './engine.js'; import os from 'node:os'; import process from 'node:process'; import childProcess from 'node:child_process'; -import { - collectFilesByExtensions, - relativizeFiles, - resolveSearchRootsFromPatterns, -} from './files.js'; +import { resolveFilesFromPatterns } from './files.js'; import * as utils from '../utils.js'; const platform = os.platform(); const SHELL_FILE_EXTENSIONS = ['.sh'] as const; +function resolveShellPatterns( + shellPatterns: readonly string[] | undefined, + defaultSearchRoots: readonly string[], +): string[] { + return shellPatterns != null && shellPatterns.length > 0 + ? [...shellPatterns] + : [...defaultSearchRoots]; +} + function createShellDomainPlugin({ defaultSearchRoots, }: { @@ -22,22 +27,17 @@ function createShellDomainPlugin({ domain: 'shell', description: 'Lint shell scripts with shellcheck when available.', detect: ({ shellPatterns }) => { - const patterns = - shellPatterns != null && shellPatterns.length > 0 - ? shellPatterns - : [...defaultSearchRoots]; - const searchRoots = resolveSearchRootsFromPatterns(patterns); - const matchedFiles = collectFilesByExtensions( - searchRoots, + const patterns = resolveShellPatterns(shellPatterns, defaultSearchRoots); + const matchedFiles = resolveFilesFromPatterns( + patterns, SHELL_FILE_EXTENSIONS, ); - const matchedRelativeFiles = relativizeFiles(matchedFiles); const hasShellcheck = utils.commandExists('shellcheck'); return { - relevant: matchedRelativeFiles.length > 0, + relevant: matchedFiles.length > 0, relevanceReason: - matchedRelativeFiles.length > 0 + matchedFiles.length > 0 ? undefined : 'No shell script files matched in effective scope.', available: hasShellcheck, @@ -45,7 +45,7 @@ function createShellDomainPlugin({ unavailableReason: hasShellcheck ? undefined : 'shellcheck not found in environment.', - matchedFiles: matchedRelativeFiles, + matchedFiles, }; }, run: ({ logger }, detection) => { diff --git a/src/utils.ts b/src/utils.ts index 93ca929..a7ddaea 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -80,6 +80,9 @@ async function runESLint({ errorOnUnmatchedPattern: false, warnIgnored: false, ignorePatterns: [], // Trust caller entirely + cache: true, + cacheLocation: '.cache/matrixai-lint/eslint/.eslintcache', + cacheStrategy: 'content', overrideConfig: parserProjectOverride, }); @@ -120,6 +123,9 @@ async function runESLint({ errorOnUnmatchedPattern: false, warnIgnored: false, ignorePatterns: ignorePats, + cache: true, + cacheLocation: '.cache/matrixai-lint/eslint/.eslintcache', + cacheStrategy: 'content', overrideConfig: parserProjectOverride, }); diff --git a/tests/domains/index.test.ts b/tests/domains/index.test.ts index 4658a65..126837d 100644 --- a/tests/domains/index.test.ts +++ b/tests/domains/index.test.ts @@ -1,6 +1,8 @@ import path from 'node:path'; import fs from 'node:fs'; +import childProcess from 'node:child_process'; import Logger, { LogLevel } from '@matrixai/logger'; +import { jest } from '@jest/globals'; import { createLintDomainRegistry, runLintDomains, @@ -533,6 +535,224 @@ describe('domain engine', () => { await fs.promises.rm(tmpRoot, { recursive: true, force: true }); } }); + + test('shell detection and run resolve globs consistently from explicit patterns', async () => { + const tmpRoot = await fs.promises.mkdtemp( + path.join(tmpDir, 'domain-shell-glob-consistency-'), + ); + + const previousCwd = process.cwd(); + const execFileSyncMock = jest + .spyOn(childProcess, 'execFileSync') + .mockImplementation( + (_file: string, _args?: readonly string[] | undefined) => + Buffer.from(''), + ); + const spawnSyncMock = jest + .spyOn(childProcess, 'spawnSync') + .mockImplementation((file: string, args?: readonly string[]) => { + const commandName = args?.[0]; + const status = + (file === 'which' || file === 'where') && commandName === 'shellcheck' + ? 0 + : 1; + + return { + pid: 0, + output: [null, null, null], + stdout: null, + stderr: null, + status, + signal: null, + error: undefined, + } as unknown as ReturnType; + }); + + try { + process.chdir(tmpRoot); + + await fs.promises.mkdir(path.join(tmpRoot, 'scripts', 'nested'), { + recursive: true, + }); + + await fs.promises.writeFile( + path.join(tmpRoot, 'scripts', 'lint.sh'), + '#!/usr/bin/env sh\necho lint\n', + 'utf8', + ); + + await fs.promises.writeFile( + path.join(tmpRoot, 'scripts', 'nested', 'test.sh'), + '#!/usr/bin/env sh\necho test\n', + 'utf8', + ); + + const registry = createBuiltInDomainRegistry({ + prettierConfigPath: path.join(tmpRoot, 'prettier.config.js'), + }); + + const decisions = await evaluateLintDomains({ + registry, + selectedDomains: new Set(['shell']), + explicitlyRequestedDomains: new Set(['shell']), + selectionSources: new Map([['shell', 'domain-flag']]), + executionOrder: ['eslint', 'shell', 'markdown'], + context: { + fix: false, + logger: testLogger, + isConfigValid: true, + shellPatterns: ['./scripts/**/*.sh'], + }, + }); + + const shellDecision = decisions.find( + (decision) => decision.domain === 'shell', + ); + const matchedFiles = (shellDecision?.detection?.matchedFiles ?? []).map( + (p) => p.split(path.sep).join(path.posix.sep), + ); + + expect(shellDecision?.plannedAction).toBe('run'); + expect(matchedFiles).toEqual( + expect.arrayContaining(['scripts/lint.sh', 'scripts/nested/test.sh']), + ); + + const hadFailure = await runLintDomains({ + registry, + selectedDomains: new Set(['shell']), + explicitlyRequestedDomains: new Set(['shell']), + selectionSources: new Map([['shell', 'domain-flag']]), + executionOrder: ['eslint', 'shell', 'markdown'], + context: { + fix: false, + logger: testLogger, + isConfigValid: true, + shellPatterns: ['./scripts/**/*.sh'], + }, + }); + + expect(hadFailure).toBe(false); + const shellcheckCall = execFileSyncMock.mock.calls.find( + ([file]) => file === 'shellcheck', + ); + const shellcheckArgs = shellcheckCall?.[1] as string[] | undefined; + const normalizedShellcheckArgs = (shellcheckArgs ?? []).map((arg) => + arg.split(path.sep).join(path.posix.sep), + ); + expect(normalizedShellcheckArgs).toEqual( + expect.arrayContaining(['scripts/lint.sh', 'scripts/nested/test.sh']), + ); + expect(normalizedShellcheckArgs).toHaveLength(2); + } finally { + spawnSyncMock.mockRestore(); + execFileSyncMock.mockRestore(); + process.chdir(previousCwd); + await fs.promises.rm(tmpRoot, { recursive: true, force: true }); + } + }); + + test('markdown detection and run resolve globs consistently from explicit patterns', async () => { + const tmpRoot = await fs.promises.mkdtemp( + path.join(tmpDir, 'domain-markdown-glob-consistency-'), + ); + + const previousCwd = process.cwd(); + const execFileSyncMock = jest + .spyOn(childProcess, 'execFileSync') + .mockImplementation( + (_file: string, _args?: readonly string[] | undefined) => + Buffer.from(''), + ); + + try { + process.chdir(tmpRoot); + + await fs.promises.mkdir(path.join(tmpRoot, 'docs', 'guides'), { + recursive: true, + }); + + await fs.promises.writeFile( + path.join(tmpRoot, 'docs', 'guides', 'a.md'), + '# A\n', + 'utf8', + ); + await fs.promises.writeFile( + path.join(tmpRoot, 'docs', 'guides', 'b.mdx'), + '# B\n', + 'utf8', + ); + + const registry = createBuiltInDomainRegistry({ + prettierConfigPath: path.join(tmpRoot, 'prettier.config.js'), + }); + + const decisions = await evaluateLintDomains({ + registry, + selectedDomains: new Set(['markdown']), + explicitlyRequestedDomains: new Set(['markdown']), + selectionSources: new Map([['markdown', 'domain-flag']]), + executionOrder: ['eslint', 'shell', 'markdown'], + context: { + fix: false, + logger: testLogger, + isConfigValid: true, + markdownPatterns: ['./docs/**/*.md*'], + }, + }); + + const markdownDecision = decisions.find( + (decision) => decision.domain === 'markdown', + ); + const matchedFiles = ( + markdownDecision?.detection?.matchedFiles ?? [] + ).map((p) => p.split(path.sep).join(path.posix.sep)); + + expect(markdownDecision?.plannedAction).toBe('run'); + expect(matchedFiles).toEqual( + expect.arrayContaining(['docs/guides/a.md', 'docs/guides/b.mdx']), + ); + + const hadFailure = await runLintDomains({ + registry, + selectedDomains: new Set(['markdown']), + explicitlyRequestedDomains: new Set(['markdown']), + selectionSources: new Map([['markdown', 'domain-flag']]), + executionOrder: ['eslint', 'shell', 'markdown'], + context: { + fix: false, + logger: testLogger, + isConfigValid: true, + markdownPatterns: ['./docs/**/*.md*'], + }, + }); + + expect(hadFailure).toBe(false); + const prettierCall = execFileSyncMock.mock.calls.find(([file, args]) => { + const argList = [...((args as readonly string[] | undefined) ?? [])]; + return ( + file === 'prettier' || + argList.some((arg) => /prettier\.cjs$/.test(arg)) + ); + }); + const prettierArgs = (prettierCall?.[1] as string[] | undefined) ?? []; + const normalizedPrettierArgs = prettierArgs.map((arg) => + arg.split(path.sep).join(path.posix.sep), + ); + expect(normalizedPrettierArgs).toEqual( + expect.arrayContaining(['docs/guides/a.md', 'docs/guides/b.mdx']), + ); + expect( + normalizedPrettierArgs.filter((arg) => arg === 'docs/guides/a.md'), + ).toHaveLength(1); + expect( + normalizedPrettierArgs.filter((arg) => arg === 'docs/guides/b.mdx'), + ).toHaveLength(1); + } finally { + execFileSyncMock.mockRestore(); + process.chdir(previousCwd); + await fs.promises.rm(tmpRoot, { recursive: true, force: true }); + } + }); }); describe('domain selection', () => {