diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f9272e..62f0490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,51 @@ # Changelog -## Unreleased - -- Deprecate `context.rawChecker`. Types overrides from `context.checker` have been updated so it can be passed to other libraries. Thanks @JoshuaKGoldberg for challenging this! +## 1.1.0 + +### Project-wide rules (experimental) + +Rules can now implement an `aggregate` function that will be called once for all files that have been linted. This allows to implement rules that require cross-file analysis, like detecting circular dependencies. + +```ts +import { type AST, core, defineConfig } from "tsl"; + +type ImportsData = { + imports: { path: string; node: AST.ImportDeclaration }[]; +}; + +export default defineConfig({ + rules: [ + ...core.all(), + { + name: "org/noCircularDependencies", + createData: (): ImportsData => ({ imports: [] }), + visitor: { + ImportDeclaration(context, node) { + context.data.imports.push({ path: toAbsolutePath(node), node }); + }, + }, + aggregate(context, files) { + // ^ { sourceFile: AST.SourceFile; data: ImportsData }[] + for (const circularEntry of getCircularDependencies(files)) { + context.report({ + message: "Circular dependency detected", + sourceFile: circularEntry.sourceFile, + node: circularEntry.data.imports[circularEntry.importIndex].node, + }); + } + }, + }, + ], +}); +``` + +`enableProjectWideRulesInIDE` is an experimental new option to enable project-wide reports in the IDE (default to false). The IDE implementation is still a work in progress, and memory leaks or performance issues may occur. + +`core/unusedExport` is a new project-wide rule available with this release. + +### Deprecate `context.rawChecker` + +Types overrides of `context.checker` have been updated so it can be passed to other libraries. `context.rawChecker` is therefore not needed anymore and has been deprecated. Thanks @JoshuaKGoldberg for challenging this! ## 1.0.28 diff --git a/README.md b/README.md index 86b5f10..20eff75 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ tsl is an extension of tsc for type-aware linting. It's designed to be used in c - Run type-aware rules [faster](https://github.com/ArnaudBarre/tsl/issues/3) than [typescript-eslint](https://typescript-eslint.io/) - Type safe config with custom rules in TypeScript +- Project-wide rules (experimental) - No [IDE caching issue](https://typescript-eslint.io/troubleshooting/faqs/general/#changes-to-one-file-are-not-reflected-when-linting-other-files-in-my-ide) - Something missing? Look at the [roadmap](https://github.com/ArnaudBarre/tsl/issues/4) or [open an issue](https://github.com/ArnaudBarre/tsl/issues/new) @@ -200,6 +201,10 @@ Like the ignore option, the `files` option test for inclusion against the file p Redeclared rules (identical name) completely replace the "base" rule, there is no merging of options. +### enableProjectWideRulesInIDE (experimental) + +Enable project-wide rules in IDE. The IDE implementation is still a work in progress, and memory leaks or performance issues may occur. Enable and report issues if you encounter any. + ## Ignore comments Rules reports can be ignored with line comments (ignore next line) or one line block comments (ignore for the whole file). @@ -271,9 +276,58 @@ export default defineConfig({ }); ``` +### Project-wide rules (added in 1.1.0, experimental) + +Rules can optionally implement an `aggregate` function that will be called once for all files that have been linted. This allows to implement rules that require cross-file analysis, like detecting circular dependencies. + +```ts +import { type AST, core, defineConfig } from "tsl"; + +type ImportsData = { + imports: { path: string; node: AST.ImportDeclaration }[]; +}; + +export default defineConfig({ + rules: [ + ...core.all(), + { + name: "org/noCircularDependencies", + createData: (): ImportsData => ({ imports: [] }), + visitor: { + ImportDeclaration(context, node) { + context.data.imports.push({ path: toAbsolutePath(node), node }); + }, + }, + aggregate(context, files) { + // ^ { sourceFile: AST.SourceFile; data: ImportsData }[] + for (const circularEntry of getCircularDependencies(files)) { + context.report({ + message: "Circular dependency detected", + sourceFile: circularEntry.sourceFile, + node: circularEntry.data.imports[circularEntry.importIndex].node, + }); + } + }, + }, + ], +}); +``` + +> [!CAUTION] +> Project-wide rules are currently not supported inside overrides + +> [!IMPORTANT] +> The list of files is only files that were linted. If you need a generated file to be in that list, but you still want to ignore all lint rules inside it, use a top level ignore comment instead of using the ignore option. + ## Core rules -Currently, the list of core rules are the type-aware lint rules I use from typescript-eslint. If you think more rules should be added, please open an issue, but to reduce the surface, only non-styling type-aware rules will be accepted. Here is the list of [typescript-eslint type aware rules](https://typescript-eslint.io/rules/?=typeInformation) with their status: +### Exclusive rules + +- unusedExport: Detect unused exports (Using experimental project-wide linting) + +### From typescript-eslint + +Currently, the ported rules are the type-aware lint rules I use from typescript-eslint. If you think more rules should be added, please open an issue, but to reduce the surface, only non-styling type-aware rules will be accepted. Here is the list of [typescript-eslint type aware rules](https://typescript-eslint.io/rules/?=typeInformation) with their status: - await-thenable: ✅ Implemented - consistent-return: 🛑 Implementation not planned, you can use `noImplicitReturns` compilerOption diff --git a/src/cli.ts b/src/cli.ts index 561a293..25a08c7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -8,7 +8,7 @@ import { type TSLDiagnostic, } from "./formatDiagnostic.ts"; import { core } from "./index.ts"; -import { initRules } from "./initRules.ts"; +import { initRules, type Report } from "./initRules.ts"; import { loadConfig } from "./loadConfig.ts"; const { values } = parseArgs({ @@ -75,11 +75,12 @@ if (values.timing) { const configStart = performance.now(); const { config } = await loadConfig(program); -const { lint, allRules, timingMaps } = await initRules( - () => program, - config ?? { rules: core.all() }, - values.timing, -); +const { lint, allRules, aggregate, getUnusedIgnoreComments, timingMaps } = + await initRules( + () => program, + config ?? { rules: core.all() }, + values.timing, + ); if (values.timing) { console.log( `Config with ${allRules.size} ${ @@ -118,39 +119,65 @@ const lintStart = performance.now(); const files = program.getSourceFiles(); +function onReport( + sourceFile: ts.SourceFile, + r: Report, + currentIdx: number | undefined, +) { + if (currentIdx === undefined) { + const lastTsDiagnostic = diagnostics.findLastIndex( + (d) => d.file === sourceFile, + ); + if (lastTsDiagnostic !== -1) { + currentIdx = lastTsDiagnostic + 1; + } else { + const sortIdx = diagnostics.findIndex((d, i) => { + const previousFile = i === 0 ? undefined : diagnostics[i - 1].file; + return ( + d.file !== undefined + && sourceFile.fileName < d.file.fileName + && (previousFile === undefined + || sourceFile.fileName > previousFile.fileName) + ); + }); + currentIdx = sortIdx === -1 ? diagnostics.length : sortIdx; + } + } else { + currentIdx++; + } + diagnostics.splice(currentIdx, 0, { + file: sourceFile, + name: r.type === "rule" ? r.rule.name : "tsl-unused-ignore", + message: r.message, + start: "node" in r ? r.node.getStart() : r.start, + length: "node" in r ? r.node.getEnd() - r.node.getStart() : r.end - r.start, + }); + return currentIdx; +} + for (const it of files) { let currentIdx: number | undefined = undefined; lint(it as unknown as SourceFile, (r) => { - if (currentIdx === undefined) { - const lastTsDiagnostic = diagnostics.findLastIndex((d) => d.file === it); - if (lastTsDiagnostic !== -1) { - currentIdx = lastTsDiagnostic + 1; - } else { - const sortIdx = diagnostics.findIndex((d, i) => { - const previousFile = i === 0 ? undefined : diagnostics[i - 1].file; - return ( - d.file !== undefined - && it.fileName < d.file.fileName - && (previousFile === undefined - || it.fileName > previousFile.fileName) - ); - }); - currentIdx = sortIdx === -1 ? diagnostics.length : sortIdx; - } - } else { - currentIdx++; - } - diagnostics.splice(currentIdx, 0, { - file: it, - name: r.type === "rule" ? r.rule.name : "tsl-unused-ignore", - message: r.message, - start: "node" in r ? r.node.getStart() : r.start, - length: - "node" in r ? r.node.getEnd() - r.node.getStart() : r.end - r.start, - }); + currentIdx = onReport(it, r, currentIdx); }); } +const aggregateReports = aggregate(); +for (const [sourceFile, reports] of aggregateReports) { + for (const report of reports) { + onReport(sourceFile, report, undefined); + } +} + +const unusedIgnoreComments = getUnusedIgnoreComments({ + includeProjectWideRules: true, +}); +for (const [sourceFile, reports] of unusedIgnoreComments) { + for (const report of reports) { + onReport(sourceFile, report, undefined); + } +} + if (values.timing) { console.log(`Lint ran in ${displayTiming(lintStart)}`); } diff --git a/src/formatDiagnostic.ts b/src/formatDiagnostic.ts index a2dcc0c..d9b0bfa 100644 --- a/src/formatDiagnostic.ts +++ b/src/formatDiagnostic.ts @@ -179,7 +179,7 @@ function formatCodeSpan( return context; } -export function formatLocation(file: SourceFile, start: number): string { +function formatLocation(file: SourceFile, start: number): string { const { line, character } = getLineAndCharacterOfPosition(file, start); const relativeFileName = displayFilename(file.fileName); let output = ""; diff --git a/src/getPlugin.ts b/src/getPlugin.ts index 4ada04e..3161de1 100644 --- a/src/getPlugin.ts +++ b/src/getPlugin.ts @@ -1,9 +1,14 @@ import { type FSWatcher, watch } from "node:fs"; -import type { CodeFixAction, Diagnostic, LanguageService } from "typescript"; -import type { SourceFile } from "./ast.ts"; -import { initRules } from "./initRules.ts"; +import type { + CodeFixAction, + Diagnostic, + LanguageService, + Program, +} from "typescript"; +import type ts from "typescript"; +import { initRules, type Report } from "./initRules.ts"; import { loadConfig } from "./loadConfig.ts"; -import type { Suggestion } from "./types.ts"; +import type { AST, Suggestion } from "./types.ts"; const UNUSED_COMMENT_CODE = "unusedComment"; @@ -17,8 +22,10 @@ export const getPlugin = async ( cleanUp(): void; }> => { const watchedFiles = new Map(); - let lint: Awaited>["lint"]; + let initRulesResult: Awaited>; let diagnosticCategory: Diagnostic["category"]; + let enableProjectWideRulesInIDE = false; + let readyForAggregate = false; const load = async () => { const start = performance.now(); @@ -36,22 +43,26 @@ export const getPlugin = async ( watchedFiles.set(configFile, watch(configFile, load)); } } - const result = await initRules( + initRulesResult = await initRules( () => languageService.getProgram()!, config ?? { rules: [] }, false, ); - lint = result.lint; diagnosticCategory = config?.diagnosticCategory === "error" ? ts.DiagnosticCategory.Error : ts.DiagnosticCategory.Warning; + readyForAggregate = false; + enableProjectWideRulesInIDE = config?.enableProjectWideRulesInIDE ?? false; (ts as any).codefix.registerCodeFix({ - errorCodes: [UNUSED_COMMENT_CODE, ...Array.from(result.allRules)], + errorCodes: [ + UNUSED_COMMENT_CODE, + ...Array.from(initRulesResult.allRules), + ], getCodeActions: () => undefined, }); log( - `tsl: Config with ${result.allRules.size} rules loaded in ${( + `tsl: Config with ${initRulesResult.allRules.size} rules loaded in ${( performance.now() - start ).toFixed(0)}ms`, ); @@ -64,19 +75,32 @@ export const getPlugin = async ( end: number; }; + const aggregateStatusByProgram = new WeakMap< + Program, + { count: number; lintedFiles: Set } + >(); + const aggregateResultsByProgram = new WeakMap< + Program, + Map + >(); const runLint = (fileName: string) => { const diagnostics: Diagnostic[] = []; const fileSuggestions: FileSuggestion[] = []; const result = { diagnostics, fileSuggestions }; if (fileName.includes("/node_modules/")) return result; - const sourceFile = languageService.getProgram()?.getSourceFile(fileName); + const program = languageService.getProgram(); + if (!program) { + log("tsl: No program"); + return result; + } + const sourceFile = program.getSourceFile(fileName); if (!sourceFile) { log("tsl: No sourceFile"); return result; } - lint(sourceFile as unknown as SourceFile, (report) => { + const onReport = (report: Report) => { switch (report.type) { case "rule": { const { message, rule, suggestions } = report; @@ -148,7 +172,48 @@ export const getPlugin = async ( break; } } - }); + }; + + initRulesResult.lint(sourceFile as unknown as AST.SourceFile, onReport); + + if (enableProjectWideRulesInIDE) { + // Aggregate if all source files have been linted once + if (!readyForAggregate) { + let aggregateStatus = aggregateStatusByProgram.get(program); + if (!aggregateStatus) { + let count = 0; + for (const it of program.getSourceFiles()) { + if (!it.fileName.includes("/node_modules/")) count++; + } + aggregateStatus = { count, lintedFiles: new Set() }; + aggregateStatusByProgram.set(program, aggregateStatus); + } + aggregateStatus.lintedFiles.add(fileName); + if (aggregateStatus.count >= aggregateStatus.lintedFiles.size) { + log("tsl: Ready for aggregate"); + readyForAggregate = true; + } + } + if (readyForAggregate) { + let aggregateReports = aggregateResultsByProgram.get(program); + if (!aggregateReports) { + aggregateReports = initRulesResult.aggregate(); + aggregateResultsByProgram.set(program, aggregateReports); + } + const reportsForFile = aggregateReports.get(sourceFile); + if (reportsForFile) { + for (const report of reportsForFile) onReport(report); + } + } + } + + const unusedIgnoreReports = initRulesResult + .getUnusedIgnoreComments({ includeProjectWideRules: readyForAggregate }) + .get(sourceFile); + if (unusedIgnoreReports) { + for (const report of unusedIgnoreReports) onReport(report); + } + return result; }; diff --git a/src/index.ts b/src/index.ts index 2392496..60a951f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,8 +32,10 @@ import { restrictTemplateExpressions } from "./rules/restrictTemplateExpressions import { returnAwait } from "./rules/returnAwait/returnAwait.ts"; import { strictBooleanExpressions } from "./rules/strictBooleanExpressions/strictBooleanExpressions.ts"; import { switchExhaustivenessCheck } from "./rules/switchExhaustivenessCheck/switchExhaustivenessCheck.ts"; +import { unusedExport } from "./rules/unusedExport/unusedExport.ts"; import type { Config, Rule } from "./types.ts"; +/* tsl-ignore core/unusedExport */ export type { AST, Checker, @@ -96,4 +98,5 @@ export const core = createRulesSet({ returnAwait, strictBooleanExpressions, switchExhaustivenessCheck, + unusedExport, }); diff --git a/src/initRules.ts b/src/initRules.ts index 3ad5f14..d5ba31b 100644 --- a/src/initRules.ts +++ b/src/initRules.ts @@ -2,6 +2,7 @@ import ts from "typescript"; import type { AnyNode, SourceFile, Visitor } from "./ast.ts"; import { getContextUtils } from "./getContextUtils.ts"; import type { + AggregateContext, AST, Checker, Config, @@ -51,20 +52,31 @@ export const initRules = async ( const baseRules = config.rules; const allRules = new Set(); + const allProjectWideRules = new Set(); for (const rule of baseRules) { allRules.add(rule.name); + if (rule.aggregate) allProjectWideRules.add(rule.name); if (showTiming) rulesTimingMap[rule.name] = 0; } if (config.overrides) { for (const override of config.overrides) { for (const rule of override.rules) { allRules.add(rule.name); + if (rule.aggregate) allProjectWideRules.add(rule.name); if (showTiming) rulesTimingMap[rule.name] = 0; } } } - type RuleWithContext = { rule: Rule; context: Context }; + type RuleWithContext = { + rule: Rule; + context: Context; + // Using a record instead of an array for fast update in plugin mode + filesForAggregate: Record< + string, + { sourceFile: SourceFile; data: unknown } + >; + }; const getRulesVisitFnCache = new Map< string /* overridesKey */, ReturnType @@ -93,6 +105,7 @@ export const initRules = async ( const rule = rulesMaps[ruleName]; rulesWithContext.push({ rule, + filesForAggregate: {}, context: { sourceFile: undefined as unknown as SourceFile, get program() { @@ -158,19 +171,41 @@ export const initRules = async ( return value; }; + const ignoreCommentsMap: Record< + string, + IgnoreComment[] | "fileIgnored" | undefined + > = {}; + const aggregateContext: AggregateContext = { + get checker() { + return getProgram().getTypeChecker() as unknown as Checker; + }, + get compilerOptions() { + return getProgram().getCompilerOptions(); + }, + get program() { + return getProgram(); + }, + utils: contextUtils, + report: undefined as unknown as AggregateContext["report"], + }; + return { allRules, lint: (sourceFile: SourceFile, onReport: (report: Report) => void) => { if (sourceFile.fileName.includes("node_modules")) return; if (config.ignore?.some((p) => sourceFile.fileName.includes(p))) return; const { fileIgnored, ignoreComments } = getIgnoreComments(sourceFile); - if (fileIgnored) return; + ignoreCommentsMap[sourceFile.fileName] = fileIgnored + ? "fileIgnored" + : ignoreComments; const { rulesWithContext, visit } = getRulesVisitFn(sourceFile.fileName); const start = showTiming ? performance.now() : 0; const reports: RuleReport[] = []; for (const { rule, context } of rulesWithContext) { + if (fileIgnored && !rule.aggregate) continue; context.sourceFile = sourceFile; context.report = (props) => { + if (fileIgnored) return; reports.push({ type: "rule", rule, ...props }); }; if (rule.createData) { @@ -186,11 +221,12 @@ export const initRules = async ( for (const report of reports) { const timingStart = showTiming ? performance.now() : 0; const start = "node" in report ? report.node.getStart() : report.start; - const line = - lineStarts.findLastIndex((lineStart) => start >= lineStart) + 1; + const line = lineStarts.findLastIndex( + (lineStart) => start >= lineStart, + ); const ignoreComment = ignoreComments.find( (comment) => - (!comment.local || comment.line === line - 1) + (!comment.local || comment.line === line) && comment.ruleNames.includes(report.rule.name), ); if (ignoreComment) { @@ -202,44 +238,114 @@ export const initRules = async ( rulesTimingMap[report.rule.name] += performance.now() - timingStart; } } - for (const comment of ignoreComments) { - if (comment.used) continue; - onReport({ - type: "ignore", - start: comment.start, - end: comment.end, - message: "Unused ignore comment", - suggestions: () => { - const lineStart = lineStarts[comment.line - 1]; - const lineEnd = lineStarts.at(comment.line); - const fixLocation = run(() => { - const fileText = sourceFile.getFullText(); - const lineText = fileText.slice(lineStart, lineEnd).trim(); - const commentText = fileText.slice(comment.start, comment.end); - if (lineText === commentText) { - return { - start: lineStart, - length: (lineEnd ?? comment.end) - lineStart, - }; - } - return { - start: comment.start, - length: comment.end - comment.start, - }; - }); - return [ - { - message: "Remove unused ignore comment", - changes: [{ ...fixLocation, newText: "" }], - }, - ]; - }, - }); + for (const ruleWithContext of rulesWithContext) { + if (!ruleWithContext.rule.aggregate) continue; + ruleWithContext.filesForAggregate[sourceFile.fileName] = { + sourceFile, + data: ruleWithContext.context.data, + }; } if (showTiming) { filesTimingMap[sourceFile.fileName] = performance.now() - start; } }, + aggregate: () => { + const map = new Map(); + const addReport = (sourceFile: ts.SourceFile, report: Report) => { + let reports = map.get(sourceFile); + if (!reports) { + reports = []; + map.set(sourceFile, reports); + } + reports.push(report); + }; + for (const { rulesWithContext } of getRulesVisitFnCache.values()) { + for (const it of rulesWithContext) { + if (!it.rule.aggregate) continue; + aggregateContext.report = (props) => { + const start = "node" in props ? props.node.getStart() : props.start; + const line = props.sourceFile + .getLineStarts() + .findLastIndex((lineStart) => start >= lineStart); + const ignoreComments = ignoreCommentsMap[props.sourceFile.fileName]; + if (ignoreComments === "fileIgnored") return; + const ignoreComment = ignoreComments?.find( + (comment) => + (!comment.local || comment.line === line) + && comment.ruleNames.includes(it.rule.name), + ); + if (ignoreComment) { + ignoreComment.used = true; + return; + } + addReport(props.sourceFile as unknown as ts.SourceFile, { + type: "rule", + rule: it.rule, + ...props, + }); + }; + it.rule.aggregate( + aggregateContext, + Object.values(it.filesForAggregate), + ); + } + } + return map; + }, + getUnusedIgnoreComments: (options: { + includeProjectWideRules: boolean; + }) => { + const map = new Map(); + for (const sourceFile of getProgram().getSourceFiles()) { + const comments = ignoreCommentsMap[sourceFile.fileName]; + if (comments === undefined || comments === "fileIgnored") continue; + const reports: Report[] = []; + for (const comment of comments) { + if (comment.used) continue; + if ( + !options.includeProjectWideRules + && comment.ruleNames.some((name) => allProjectWideRules.has(name)) + ) { + // Don't generate false positives reports if project-wide rules didn't run + continue; + } + reports.push({ + type: "ignore", + start: comment.start, + end: comment.end, + message: "Unused ignore comment", + suggestions: () => { + const lineStarts = sourceFile.getLineStarts(); + const lineStart = lineStarts[comment.line - 1]; + const lineEnd = lineStarts.at(comment.line); + const fixLocation = run(() => { + const fileText = sourceFile.getFullText(); + const lineText = fileText.slice(lineStart, lineEnd).trim(); + const commentText = fileText.slice(comment.start, comment.end); + if (lineText === commentText) { + return { + start: lineStart, + length: (lineEnd ?? comment.end) - lineStart, + }; + } + return { + start: comment.start, + length: comment.end - comment.start, + }; + }); + return [ + { + message: "Remove unused ignore comment", + changes: [{ ...fixLocation, newText: "" }], + }, + ]; + }, + }); + } + if (reports.length) map.set(sourceFile, reports); + } + return map; + }, timingMaps: showTiming ? { Rule: rulesTimingMap, File: filesTimingMap } : undefined, diff --git a/src/loadConfig.ts b/src/loadConfig.ts index ea36f80..249468a 100644 --- a/src/loadConfig.ts +++ b/src/loadConfig.ts @@ -74,7 +74,7 @@ export const loadConfig = async ( }; }; -export const findConfigPath = (program: ts.Program) => { +const findConfigPath = (program: ts.Program) => { let dir = program.getCurrentDirectory(); const { root } = path.parse(dir); let configPath: string | undefined = undefined; diff --git a/src/ruleTester.ts b/src/ruleTester.ts index 27237b3..b8f5e90 100644 --- a/src/ruleTester.ts +++ b/src/ruleTester.ts @@ -4,6 +4,7 @@ import { getContextUtils } from "./getContextUtils.ts"; import type { AST, Checker, Context, ReportDescriptor, Rule } from "./types.ts"; import { visitorEntries } from "./visitorEntries.ts"; +// tsl-ignore core/unusedExport export function print(...args: any[]) { console.log(...args.map((value) => transform(value, new Set()))); } @@ -67,18 +68,24 @@ type CaseProps Rule> = { options?: Parameters[0]; code: string; }; +type ProjectWideCaseProps Rule> = + { + compilerOptions?: ts.CompilerOptions; + options?: Parameters[0]; + files: { fileName: `${string}.${"ts" | "tsx"}`; code: string }[]; + }; type ErrorReport = { + fileName?: string; message: string; line?: number; column?: number; endLine?: number; endColumn?: number; - suggestions?: { message: string; output?: string }[]; + suggestions?: { message: string; output: string }[]; }; type SetupCase Rule> = { compilerOptionsKey: string; - fileName: string; - code: string; + files: { path: string; fileName: string; code: string }[]; options?: Parameters[0]; isValid: boolean; index: number; @@ -101,25 +108,26 @@ const defaultCompilerOptions = { const typeFocus = process.argv[3]; const indexFocus = process.argv[4]; -export type ValidTestCase Rule> = +export type ValidTestCase Rule> = + | ProjectWideCaseProps | CaseProps | string; export type InvalidTestCase< TRule extends (options?: unknown) => Rule, -> = CaseProps & { - error?: string; - errors?: ( - | { - message: string; - line?: number; - column?: number; - endColumn?: number; - endLine?: number; - suggestions?: { message: string; output: string }[]; - } - | [message: string, line?: number, column?: number] - )[]; -}; +> = + | (CaseProps + & ( + | { error: string } + | { + errors: ( + | Omit + | [message: string, line?: number, column?: number] + )[]; + } + )) + | (ProjectWideCaseProps & { + errors: ErrorReport[]; + }); /** * Output API is still a bit raw (it prints differences to the console and returns a 'hasError' boolean) @@ -143,16 +151,35 @@ export const ruleTester = Rule>({ const cases: SetupCase[] = []; const setupCase = ( - caseProps: CaseProps, + caseProps: CaseProps | ProjectWideCaseProps, isValid: boolean, index: number, errors: ErrorReport[] | null, ) => { - const useTSX = tsx ? caseProps.tsx !== false : caseProps.tsx === true; - const fileName = `${isValid ? "valid" : "invalid"}-${index}.${ - useTSX ? "tsx" : "ts" - }`; - filesMap.set(fileName, caseProps.code); + const useTSX = + "files" in caseProps + ? caseProps.files.some((file) => file.fileName.endsWith(".tsx")) + : tsx + ? caseProps.tsx !== false + : caseProps.tsx === true; + const caseFolder = `${isValid ? "valid" : "invalid"}-${index}`; + const files = + "files" in caseProps + ? caseProps.files.map((file) => ({ + path: `${caseFolder}/${file.fileName}`, + fileName: file.fileName, + code: file.code, + })) + : [ + { + path: `${caseFolder}/file.${useTSX ? "tsx" : "ts"}`, + fileName: `file.${useTSX ? "tsx" : "ts"}`, + code: caseProps.code, + }, + ]; + for (const file of files) { + filesMap.set(file.path, file.code); + } const compilerOptionsInput = caseProps.compilerOptions !== undefined || useTSX ? { @@ -168,10 +195,10 @@ export const ruleTester = Rule>({ const compilerOptionsKey = JSON.stringify(compilerOptionsInput); const current = compilerOptionsToFiles.get(compilerOptionsKey); if (current) { - current.push(fileName); + current.push(...files.map((f) => f.path)); } else { compilerOptionsToFiles.set(compilerOptionsKey, [ - fileName, + ...files.map((f) => f.path), ...compilerOptionsInput.lib.map( (lib) => `node_modules/typescript/lib/lib.${lib}.d.ts`, ), @@ -179,8 +206,7 @@ export const ruleTester = Rule>({ } cases.push({ compilerOptionsKey, - fileName, - code: caseProps.code, + files, options: caseProps.options, isValid, index, @@ -198,15 +224,12 @@ export const ruleTester = Rule>({ for (const [index, invalidCase] of invalid.entries()) { if (typeFocus && typeFocus !== "invalid") continue; if (indexFocus && indexFocus !== index.toString()) continue; - const errors = invalidCase.errors - ? invalidCase.errors.map((e) => - Array.isArray(e) - ? { message: e[0], line: e[1], column: e[2], suggestions: [] } - : e, - ) - : invalidCase.error + const errors = + "error" in invalidCase ? [{ message: invalidCase.error }] - : []; + : invalidCase.errors.map((e) => + Array.isArray(e) ? { message: e[0], line: e[1], column: e[2] } : e, + ); if (errors.length === 0) { throw new Error(`Invalid case ${index} has no errors`); } @@ -272,45 +295,65 @@ export const ruleTester = Rule>({ const program = compilerOptionsToProgram.get(caseProps.compilerOptionsKey)!; const compilerOptions = program.getCompilerOptions(); const reports: ReportDescriptor[] = []; - const sourceFile = program.getSourceFile( - caseProps.fileName, - ) as unknown as SourceFile; - const context: Context = { - sourceFile, - program, - get checker() { - return program.getTypeChecker() as unknown as Checker; - }, - get rawChecker() { - return program.getTypeChecker(); - }, - compilerOptions, - utils: getContextUtils(() => program), - report(descriptor) { - reports.push(descriptor); + const filesData: { sourceFile: SourceFile; data: unknown }[] = []; + for (const file of caseProps.files) { + const sourceFile = program.getSourceFile( + file.path, + ) as unknown as SourceFile; + const context: Context = { + sourceFile, + program, + get checker() { + return program.getTypeChecker() as unknown as Checker; + }, + get rawChecker() { + return program.getTypeChecker(); + }, + compilerOptions, + utils: getContextUtils(() => program), + report(descriptor) { + reports.push(descriptor); + }, + data: undefined, + }; + if (rule.createData) context.data = rule.createData(context); + const visit = (node: AST.AnyNode) => { + const nodeType = visitorEntries.find((e) => e[0] === node.kind)?.[1]; + if (nodeType) { + rule.visitor[nodeType]?.(context, node as any); + } + // @ts-expect-error + node.forEachChild(visit); + if (nodeType) { + rule.visitor[`${nodeType}_exit` as keyof Visitor]?.( + context, + node as any, + ); + } + }; + visit(sourceFile); + filesData.push({ sourceFile, data: context.data }); + } + rule.aggregate?.( + { + program, + checker: program.getTypeChecker() as unknown as Checker, + compilerOptions, + utils: getContextUtils(() => program), + report(descriptor) { + reports.push(descriptor); + }, }, - data: undefined, - }; - if (rule.createData) context.data = rule.createData(context); - const visit = (node: AST.AnyNode) => { - const nodeType = visitorEntries.find((e) => e[0] === node.kind)?.[1]; - if (nodeType) { - rule.visitor[nodeType]?.(context, node as any); - } - // @ts-expect-error - node.forEachChild(visit); - if (nodeType) { - rule.visitor[`${nodeType}_exit` as keyof Visitor]?.( - context, - node as any, - ); - } - }; - visit(sourceFile); + filesData, + ); + const displayCode = + caseProps.files.length > 1 + ? caseProps.files.map((file) => file.fileName).join(", ") + : caseProps.files[0].code; if (caseProps.isValid) { if (reports.length !== 0) { console.error( - `Reports for valid case ${caseProps.index} (${caseProps.code})`, + `Reports for valid case ${caseProps.index} (${displayCode})`, ); hasError = true; for (const report of reports) { @@ -320,7 +363,7 @@ export const ruleTester = Rule>({ } else { if (reports.length === 0) { console.error( - `No reports for invalid case ${caseProps.index} (${caseProps.code})`, + `No reports for invalid case ${caseProps.index} (${displayCode})`, ); hasError = true; } else { @@ -337,7 +380,7 @@ export const ruleTester = Rule>({ ) => { if (!introLogged) { console.error( - `Report(s) mismatch for invalid case ${caseProps.index} (${caseProps.code})`, + `Report(s) mismatch for invalid case ${caseProps.index} (${displayCode})`, ); hasError = true; introLogged = true; @@ -363,6 +406,22 @@ export const ruleTester = Rule>({ continue; } if (!expected || !got) continue; + const fileWithError = expected.fileName + ? caseProps.files.find( + (file) => file.fileName === expected.fileName, + ) + : caseProps.files[0]; + if (fileWithError === undefined) { + log( + " filename", + `One of ${caseProps.files.map((file) => file.fileName).join(", ")}`, + expected.fileName, + ); + continue; + } + const sourceFile = program.getSourceFile( + fileWithError.path, + ) as unknown as SourceFile; if (expected.line !== undefined) { const gotStart = "node" in got ? got.node.getStart() : got.start; const gotLine = @@ -452,7 +511,7 @@ export const ruleTester = Rule>({ const gotOutput = gotChanges.reduceRight( (acc, it) => acc.slice(0, it.start) + it.newText + acc.slice(it.end), - caseProps.code, + fileWithError.code, ); if (expectedSuggestion.output !== gotOutput) { log( diff --git a/src/rules/_utils/getOperatorPrecedence.ts b/src/rules/_utils/getOperatorPrecedence.ts index 1317908..15610ac 100644 --- a/src/rules/_utils/getOperatorPrecedence.ts +++ b/src/rules/_utils/getOperatorPrecedence.ts @@ -392,7 +392,7 @@ export function getOperatorPrecedence( } } -export function getBinaryOperatorPrecedence(kind: SyntaxKind): number { +function getBinaryOperatorPrecedence(kind: SyntaxKind): number { switch (kind) { case SyntaxKind.MinusToken: case SyntaxKind.PlusToken: diff --git a/src/rules/noMisusedPromises/noMisusedPromises.ts b/src/rules/noMisusedPromises/noMisusedPromises.ts index a575074..0a95d34 100644 --- a/src/rules/noMisusedPromises/noMisusedPromises.ts +++ b/src/rules/noMisusedPromises/noMisusedPromises.ts @@ -312,7 +312,7 @@ function checkVariableDeclaration( } } -export function checkPropertyAssignment( +function checkPropertyAssignment( context: Context, node: AST.PropertyAssignment, ) { @@ -335,7 +335,7 @@ export function checkPropertyAssignment( } } -export function checkShorthandPropertyAssignment( +function checkShorthandPropertyAssignment( context: Context, node: AST.ShorthandPropertyAssignment, ) { @@ -349,7 +349,7 @@ export function checkShorthandPropertyAssignment( } } -export function checkMethodDeclaration( +function checkMethodDeclaration( context: Context, node: AST.MethodDeclaration, ) { diff --git a/src/rules/noUnnecessaryCondition/noUnnecessaryCondition.ts b/src/rules/noUnnecessaryCondition/noUnnecessaryCondition.ts index 8de043e..9abc750 100644 --- a/src/rules/noUnnecessaryCondition/noUnnecessaryCondition.ts +++ b/src/rules/noUnnecessaryCondition/noUnnecessaryCondition.ts @@ -618,7 +618,7 @@ function checkCallExpression( * Inspect a call expression to see if it's a call to an assertion function. * If it is, return the node of the argument that is asserted and other useful info. */ -export function findTypeGuardAssertedArgument( +function findTypeGuardAssertedArgument( context: Context, node: AST.CallExpression, ): { argument: AST.Expression; asserts: boolean; type: ts.Type } | undefined { @@ -870,7 +870,7 @@ function checkOptionalChain( } // Truthiness utilities -export function isPossiblyFalsy(type: ts.Type): boolean { +function isPossiblyFalsy(type: ts.Type): boolean { return isTypeRecurser(type, (t) => { return t.isLiteral() ? !Boolean(getValueOfLiteralType(t)) diff --git a/src/rules/unusedExport/unusedExport.test.ts b/src/rules/unusedExport/unusedExport.test.ts new file mode 100644 index 0000000..096cf1f --- /dev/null +++ b/src/rules/unusedExport/unusedExport.test.ts @@ -0,0 +1,113 @@ +import { expect, test } from "bun:test"; +import { ruleTester, type ValidTestCase } from "../../ruleTester.ts"; +import { messages, unusedExport } from "./unusedExport.ts"; + +test("unusedExport", () => { + const hasError = ruleTester({ + ruleFn: unusedExport, + valid: [ + ...[ + "export const foo = 1;", + "export const foo = () => 1;", + "const foo = 1; export { foo };", + "export function foo() { return 1; }", + "export class foo { }", + ].map( + (code): ValidTestCase => ({ + files: [ + { fileName: "file.ts", code }, + { fileName: "file2.ts", code: `import { foo } from "./file.ts";` }, + ], + }), + ), + ...[ + "const foo = 1; export default foo;", + "export default function foo() { return 1; }", + "export default class foo { }", + ].map( + (code): ValidTestCase => ({ + files: [ + { fileName: "file.ts", code }, + { fileName: "file2.ts", code: `import bar from "./file.ts";` }, + ], + }), + ), + { + files: [ + { + fileName: "file.ts", + code: "export const foo = 1; export const bar = 2;", + }, + { fileName: "file2.ts", code: `import * as utils from "./file.ts";` }, + ], + }, + ], + invalid: [ + { + files: [ + { + fileName: "file.ts", + code: `export const foo = 1;\nexport const bar = 2;`, + }, + { fileName: "file2.ts", code: `import { foo } from "./file.ts";` }, + ], + errors: [ + { + fileName: "file.ts", + message: messages.unusedNamedExport, + line: 2, + }, + ], + }, + { + files: [ + { + fileName: "file.ts", + code: `export const foo = 1;\nconst bar = 2; export default bar;`, + }, + { fileName: "file2.ts", code: `import { foo } from "./file.ts";` }, + ], + errors: [ + { + fileName: "file.ts", + message: messages.unusedDefaultExport, + line: 2, + }, + ], + }, + { + files: [ + { + fileName: "file.ts", + code: `export const foo = 1;\nconst bar = 2; export { bar };`, + }, + { fileName: "file2.ts", code: `import { foo } from "./file.ts";` }, + ], + errors: [ + { + fileName: "file.ts", + message: messages.unusedNamedExport, + line: 2, + }, + ], + }, + { + files: [ + { + fileName: "file.ts", + code: `export const foo = 1;\nexport function bar() { return 2; };`, + }, + { fileName: "file2.ts", code: `import { foo } from "./file.ts";` }, + ], + errors: [ + { + fileName: "file.ts", + message: messages.unusedNamedExport, + line: 2, + }, + ], + }, + ], + }); + expect(hasError).toEqual(false); +}); diff --git a/src/rules/unusedExport/unusedExport.ts b/src/rules/unusedExport/unusedExport.ts new file mode 100644 index 0000000..3a2556b --- /dev/null +++ b/src/rules/unusedExport/unusedExport.ts @@ -0,0 +1,146 @@ +import { dirname, join } from "node:path"; +import ts, { SyntaxKind } from "typescript"; +import { defineRule, hasModifier } from "../_utils/index.ts"; + +export const messages = { + unusedDefaultExport: + "This default export is never used. 'export default' can be removed.", + unusedNamedExport: + "This named export is never used. The export keyword can be removed.", +}; + +type ImportUsage = { named: string[]; default: boolean; star: boolean }; +type Data = { + path: string; + imports: Record; + exports: { + named: { name: string; node: ts.Node }[]; + default: { node: ts.Node } | { start: number; end: number } | undefined; + }; + usages: ImportUsage[]; +}; + +export const unusedExport = defineRule(() => ({ + name: "core/unusedExport", + createData: (context): Data => ({ + path: context.sourceFile.fileName, + imports: {}, + exports: { named: [], default: undefined }, + usages: [], + }), + visitor: { + SourceFile(context) { + for (const el of context.sourceFile.statements) { + switch (el.kind) { + case SyntaxKind.ExportDeclaration: + if (el.exportClause?.kind === SyntaxKind.NamedExports) { + for (const e of el.exportClause.elements) { + context.data.exports.named.push({ + name: e.name.text, + node: e.name, + }); + } + } + break; + case SyntaxKind.ExportAssignment: + context.data.exports.default = { + start: el.getStart(), + end: el.expression.getStart() - 1, + }; + break; + case SyntaxKind.VariableStatement: + if (hasModifier(el, SyntaxKind.ExportKeyword)) { + for (const v of el.declarationList.declarations) { + if (v.name.kind !== SyntaxKind.Identifier) continue; // Should not be possible + context.data.exports.named.push({ + name: v.name.text, + node: v.name, + }); + } + } + break; + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ClassDeclaration: + if (hasModifier(el, SyntaxKind.ExportKeyword)) { + const defaultModifier = el.modifiers?.find( + (m) => m.kind === SyntaxKind.DefaultKeyword, + ); + if (defaultModifier) { + context.data.exports.default = { node: defaultModifier }; + } else { + if (el.name) { + context.data.exports.named.push({ + name: el.name.text, + node: el.name, + }); + } + } + } + break; + default: + break; + } + } + }, + ImportDeclaration(context, node) { + if (node.moduleSpecifier.kind !== SyntaxKind.StringLiteral) return; + const path = node.moduleSpecifier.text; + if (!path.startsWith(".")) return; + if (!path.endsWith(".ts") && !path.endsWith(".tsx")) return; + if (!node.importClause) return; + const fullPath = join(dirname(context.sourceFile.fileName), path); + context.data.imports[fullPath] ??= { + named: [], + default: false, + star: false, + }; + if (node.importClause.name) { + context.data.imports[fullPath].default = true; + } + if (node.importClause.namedBindings?.kind === SyntaxKind.NamedImports) { + for (const el of node.importClause.namedBindings.elements) { + context.data.imports[fullPath].named.push(el.name.text); + } + } + if ( + node.importClause.namedBindings?.kind === SyntaxKind.NamespaceImport + ) { + context.data.imports[fullPath].star = true; + } + }, + }, + aggregate(context, files) { + const map: Record = + Object.fromEntries(files.map((file) => [file.data.path, file.data])); + for (const file of files) { + for (const path in file.data.imports) { + const data = map[path]; + if (data) data.usages.push(file.data.imports[path]); + } + } + for (const file of files) { + if (file.data.exports.default) { + if (file.sourceFile.fileName.includes(".config.")) continue; + if (!file.data.usages.some((u) => u.default)) { + context.report({ + message: messages.unusedDefaultExport, + sourceFile: file.sourceFile, + ...file.data.exports.default, + }); + } + } + for (const named of file.data.exports.named) { + const used = file.data.usages.find( + (u) => u.star || u.named.includes(named.name), + ); + if (!used) { + context.report({ + message: messages.unusedNamedExport, + sourceFile: file.sourceFile, + node: named.node, + }); + } + } + } + }, +})); diff --git a/src/types.ts b/src/types.ts index de9bfed..0d126fa 100644 --- a/src/types.ts +++ b/src/types.ts @@ -66,6 +66,13 @@ export type Config = { */ rules: Rule[]; }[]; + /** + * The IDE implementation is still a work in progress, + * and memory leaks or performance issues may occur. + * @default false + * @experimental + */ + enableProjectWideRulesInIDE?: boolean; }; export type Rule = { @@ -91,6 +98,20 @@ export type Rule = { * ``` */ visitor: AST.Visitor; + /** + * Called once after all files have been visited. Can be used to generate new reports that + * require cross-file analysis like validating imports or usages. + * @example + * ```ts + * aggregate: (context, files) => { + * + * } + * ``` + */ + aggregate?: ( + context: AggregateContext, + files: { sourceFile: AST.SourceFile; data: Data }[], + ) => void; }; export type Checker = Omit< @@ -222,3 +243,25 @@ export type Context = { */ data: Data; }; + +export type ReportDescriptorWithSourceFile = ReportDescriptor & { + sourceFile: AST.SourceFile; +}; +export type AggregateContext = { + program: Context["program"]; + checker: Context["checker"]; + compilerOptions: Context["compilerOptions"]; + utils: Context["utils"]; + /** + * Report a diagnostic + * @example + * ```ts + * context.report({ + * sourceFile: sourceFile, + * node: node.expression, + * message: "Foo is unused.", + * }); + * ``` + */ + report(descriptor: ReportDescriptorWithSourceFile): void; +}; diff --git a/tsl.config.ts b/tsl.config.ts index 11f7e75..b08bcc4 100644 --- a/tsl.config.ts +++ b/tsl.config.ts @@ -1,6 +1,7 @@ import { core, defineConfig } from "./src/index.ts"; export default defineConfig({ + enableProjectWideRulesInIDE: true, rules: [ ...core.all(), core.noConfusingVoidExpression({ ignoreArrowShorthand: true }),