Skip to content

Commit 283ddec

Browse files
Report unused suppressions (#10805)
<img width="299" height="143" alt="image" src="https://github.com/user-attachments/assets/d401c48a-ac99-47f2-b2c3-ecf0a170bdbe" />
1 parent 9728385 commit 283ddec

8 files changed

Lines changed: 414 additions & 101 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
changeKind: feature
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
Dim unused `#suppress` directives for available compiler and library diagnostics in editor scenarios.
8+
9+
```typespec
10+
#suppress "deprecated" "old suppression"
11+
model Widget {}
12+
```

packages/compiler/src/core/messages.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,4 +1145,5 @@ const diagnostics = {
11451145
} as const;
11461146

11471147
export type CompilerDiagnostics = TypeOfDiagnostics<typeof diagnostics>;
1148+
export const compilerDiagnosticCodes = new Set(Object.keys(diagnostics));
11481149
export const { createDiagnostic, reportDiagnostic } = createDiagnosticCreator(diagnostics);

packages/compiler/src/core/program.ts

Lines changed: 17 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,14 @@ import {
4141
} from "./source-loader.js";
4242
import { createStateAccessors } from "./state-accessors.js";
4343
import { ComplexityStats, RuntimeStats, Stats } from "./stats.js";
44+
import {
45+
SuppressionTracker,
46+
createSuppressionTracker,
47+
findDirectiveSuppressingOnNode,
48+
} from "./suppression-tracking.js";
4449
import {
4550
CompilerHost,
4651
Diagnostic,
47-
Directive,
48-
DirectiveExpressionNode,
4952
EmitContext,
5053
EmitterFunc,
5154
Entity,
@@ -64,7 +67,6 @@ import {
6467
Sym,
6568
SymbolFlags,
6669
SymbolTable,
67-
SyntaxKind,
6870
TemplateInstanceTarget,
6971
Tracer,
7072
Type,
@@ -118,6 +120,8 @@ export interface Program {
118120

119121
/** @internal */
120122
reportDuplicateSymbols(symbols: SymbolTable | undefined): void;
123+
/** @internal */
124+
readonly suppressionTracker: SuppressionTracker | undefined;
121125

122126
getGlobalNamespaceType(): Namespace;
123127

@@ -218,9 +222,11 @@ async function createProgram(
218222
const emitters: EmitterRef[] = [];
219223
const requireImports = new Map<string, string>();
220224
const complexityStats: ComplexityStats = {} as any;
221-
let sourceResolution: SourceResolution;
225+
let sourceResolution: SourceResolution = undefined!;
222226
let error = false;
223227
let continueToNextStage = true;
228+
// eslint-disable-next-line prefer-const -- reassigned after source resolution
229+
let suppressionTracker: SuppressionTracker | undefined;
224230

225231
const logger = createLogger({ sink: host.logSink });
226232
const tracer = createTracer(logger, { filter: options.trace });
@@ -250,6 +256,9 @@ async function createProgram(
250256
reportDiagnostic,
251257
reportDiagnostics,
252258
reportDuplicateSymbols,
259+
get suppressionTracker() {
260+
return suppressionTracker;
261+
},
253262
hasError() {
254263
return error;
255264
},
@@ -295,6 +304,8 @@ async function createProgram(
295304
// let GC reclaim old program, we do not reuse it beyond this point.
296305
oldProgram = undefined;
297306

307+
suppressionTracker = createSuppressionTracker(sourceResolution);
308+
298309
const resolver = (program.resolver = createResolver(program));
299310
runtimeStats.resolver = perf.time(() => resolver.resolveProgram());
300311

@@ -821,6 +832,7 @@ async function createProgram(
821832
if (suppressing) {
822833
if (diagnostic.severity === "error") {
823834
// Cannot suppress errors.
835+
suppressionTracker?.markUsed(suppressing.node);
824836
diagnostics.push({
825837
severity: "error",
826838
code: "suppress-error",
@@ -830,59 +842,13 @@ async function createProgram(
830842

831843
return false;
832844
} else {
845+
suppressionTracker?.markUsed(suppressing.node);
833846
return true;
834847
}
835848
}
836849
return false;
837850
}
838851

839-
function findDirectiveSuppressingOnNode(code: string, node: Node): Directive | undefined {
840-
let current: Node | undefined = node;
841-
do {
842-
if (current.directives) {
843-
const directive = findDirectiveSuppressingCode(code, current.directives);
844-
if (directive) {
845-
return directive;
846-
}
847-
}
848-
} while ((current = current.parent));
849-
return undefined;
850-
}
851-
852-
/**
853-
* Returns the directive node that is suppressing this code.
854-
* @param code Code to check for suppression.
855-
* @param directives List of directives.
856-
* @returns Directive suppressing this code if found, `undefined` otherwise
857-
*/
858-
function findDirectiveSuppressingCode(
859-
code: string,
860-
directives: readonly DirectiveExpressionNode[],
861-
): Directive | undefined {
862-
for (const directive of directives.map((x) => parseDirective(x))) {
863-
if (directive.name === "suppress") {
864-
if (directive.code === code) {
865-
return directive;
866-
}
867-
}
868-
}
869-
return undefined;
870-
}
871-
872-
function parseDirective(node: DirectiveExpressionNode): Directive {
873-
const args = node.arguments.map((x) => {
874-
return x.kind === SyntaxKind.Identifier ? x.sv : x.value;
875-
});
876-
switch (node.target.sv) {
877-
case "suppress":
878-
return { name: "suppress", code: args[0], message: args[1], node };
879-
case "deprecated":
880-
return { name: "deprecated", message: args[0], node };
881-
default:
882-
throw new Error("Unexpected directive name.");
883-
}
884-
}
885-
886852
function getNode(target: Node | Entity | Sym | TemplateInstanceTarget): Node | undefined {
887853
if (!("kind" in target) && !("valueKind" in target) && !("entityKind" in target)) {
888854
// TemplateInstanceTarget
Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
import { defineCodeFix, getSourceLocation } from "./diagnostics.js";
2+
import { builtInLinterLibraryName } from "./linter.js";
3+
import { compilerDiagnosticCodes } from "./messages.js";
4+
import { visitChildren } from "./parser.js";
5+
import { SourceResolution } from "./source-loader.js";
6+
import {
7+
CodeFix,
8+
Directive,
9+
DirectiveExpressionNode,
10+
Node,
11+
SuppressDirective,
12+
SyntaxKind,
13+
} from "./types.js";
14+
15+
export interface UnusedSuppression {
16+
directive: SuppressDirective;
17+
}
18+
19+
export interface SuppressionTracker {
20+
markUsed(directiveNode: DirectiveExpressionNode): void;
21+
getUnusedSuppressions(): UnusedSuppression[];
22+
}
23+
24+
export function createSuppressionTracker(sourceResolution: SourceResolution): SuppressionTracker {
25+
const suppressions = collectSuppressions(sourceResolution);
26+
27+
return {
28+
markUsed(directiveNode) {
29+
const suppression = suppressions.get(directiveNode);
30+
if (suppression) {
31+
suppression.used = true;
32+
}
33+
},
34+
getUnusedSuppressions() {
35+
const unused: UnusedSuppression[] = [];
36+
for (const suppression of suppressions.values()) {
37+
if (suppression.used) {
38+
continue;
39+
}
40+
41+
const availability = getSuppressionSourceAvailability(
42+
suppression.directive.code,
43+
sourceResolution,
44+
);
45+
if (availability === "unavailable") {
46+
continue;
47+
}
48+
49+
unused.push({ directive: suppression.directive });
50+
}
51+
return unused;
52+
},
53+
};
54+
}
55+
56+
interface SuppressionRecord {
57+
directive: SuppressDirective;
58+
used: boolean;
59+
}
60+
61+
function collectSuppressions(
62+
sourceResolution: SourceResolution,
63+
): Map<DirectiveExpressionNode, SuppressionRecord> {
64+
const suppressions = new Map<DirectiveExpressionNode, SuppressionRecord>();
65+
for (const script of sourceResolution.sourceFiles.values()) {
66+
if (sourceResolution.locationContexts.get(script.file)?.type !== "project") {
67+
continue;
68+
}
69+
70+
visit(script);
71+
}
72+
73+
return suppressions;
74+
75+
function visit(node: Node) {
76+
for (const directiveNode of node.directives ?? []) {
77+
const directive = parseDirective(directiveNode);
78+
if (directive?.name === "suppress") {
79+
suppressions.set(directive.node, { directive, used: false });
80+
}
81+
}
82+
83+
visitChildren(node, visit);
84+
}
85+
}
86+
87+
function getSuppressionSourceAvailability(
88+
code: string,
89+
sourceResolution: SourceResolution,
90+
): "available" | "unavailable" {
91+
if (
92+
compilerDiagnosticCodes.has(code) ||
93+
matchesDiagnosticSource(code, builtInLinterLibraryName)
94+
) {
95+
return "available";
96+
}
97+
98+
for (const libraryName of sourceResolution.loadedLibraries.keys()) {
99+
if (matchesDiagnosticSource(code, libraryName)) {
100+
return "available";
101+
}
102+
}
103+
104+
return "unavailable";
105+
}
106+
107+
function matchesDiagnosticSource(code: string, source: string): boolean {
108+
return code.startsWith(`${source}/`);
109+
}
110+
111+
export function findDirectiveSuppressingOnNode(code: string, node: Node): Directive | undefined {
112+
let current: Node | undefined = node;
113+
do {
114+
if (current.directives) {
115+
const directive = findDirectiveSuppressingCode(code, current.directives);
116+
if (directive) {
117+
return directive;
118+
}
119+
}
120+
} while ((current = current.parent));
121+
return undefined;
122+
}
123+
124+
/**
125+
* Returns the directive node that is suppressing this code.
126+
* @param code Code to check for suppression.
127+
* @param directives List of directives.
128+
* @returns Directive suppressing this code if found, `undefined` otherwise
129+
*/
130+
export function findDirectiveSuppressingCode(
131+
code: string,
132+
directives: readonly DirectiveExpressionNode[],
133+
): Directive | undefined {
134+
for (const directiveNode of directives) {
135+
const directive = parseDirective(directiveNode);
136+
if (directive?.name === "suppress") {
137+
if (directive.code === code) {
138+
return directive;
139+
}
140+
}
141+
}
142+
return undefined;
143+
}
144+
145+
export function parseDirective(node: DirectiveExpressionNode): Directive | undefined {
146+
const args = node.arguments.map((x) => {
147+
return x.kind === SyntaxKind.Identifier ? x.sv : x.value;
148+
});
149+
switch (node.target.sv) {
150+
case "suppress":
151+
if (typeof args[0] !== "string") {
152+
return undefined;
153+
}
154+
return { name: "suppress", code: args[0], message: args[1] ?? "", node };
155+
case "deprecated":
156+
if (typeof args[0] !== "string") {
157+
return undefined;
158+
}
159+
return { name: "deprecated", message: args[0], node };
160+
default:
161+
return undefined;
162+
}
163+
}
164+
165+
export function createRemoveUnusedSuppressionCodeFix(node: DirectiveExpressionNode): CodeFix {
166+
return defineCodeFix({
167+
id: "remove-unused-suppression",
168+
label: "Remove unused suppression",
169+
fix: (context) => {
170+
const location = getSourceLocation(node);
171+
const text = location.file.text;
172+
const lineStart = text.lastIndexOf("\n", location.pos - 1) + 1;
173+
const textBeforeDirective = text.slice(lineStart, location.pos);
174+
175+
if (textBeforeDirective.trim() !== "") {
176+
return context.replaceText(location, "");
177+
}
178+
179+
let end = location.end;
180+
while (end < text.length && (text[end] === " " || text[end] === "\t")) {
181+
end++;
182+
}
183+
if (text[end] === "\r" && text[end + 1] === "\n") {
184+
end += 2;
185+
} else if (text[end] === "\n" || text[end] === "\r") {
186+
end++;
187+
}
188+
189+
return context.replaceText({ ...location, pos: lineStart, end }, "");
190+
},
191+
});
192+
}

packages/compiler/src/server/serverlib.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import {
7272
import { type Program } from "../core/program.js";
7373
import { skipTrivia, skipWhiteSpace } from "../core/scanner.js";
7474
import { createSourceFile, getSourceFileKindFromExt } from "../core/source-file.js";
75+
import { createRemoveUnusedSuppressionCodeFix } from "../core/suppression-tracking.js";
7576
import {
7677
AugmentDecoratorStatementNode,
7778
CodeFixEdit,
@@ -844,6 +845,38 @@ export function createServer(
844845
}
845846
}
846847

848+
// Report unused suppressions as hints with faded-out styling
849+
for (const { directive } of program.suppressionTracker?.getUnusedSuppressions() ?? []) {
850+
const unusedSuppressionDiagnostic: Diagnostic = {
851+
code: "unused-suppression",
852+
severity: "warning",
853+
message: `Suppression for "${directive.code}" is unused.`,
854+
target: directive.node,
855+
codefixes: [createRemoveUnusedSuppressionCodeFix(directive.node)],
856+
};
857+
const results = convertDiagnosticToLsp(
858+
fileService,
859+
program,
860+
document,
861+
unusedSuppressionDiagnostic,
862+
);
863+
for (const [diagnostic, diagDocument] of results) {
864+
diagnostic.tags = [DiagnosticTag.Unnecessary];
865+
diagnostic.severity = DiagnosticSeverity.Hint;
866+
diagnostic.data = {
867+
id: diagnosticIdCounter++,
868+
file: diagDocument.uri,
869+
};
870+
const diagnostics = diagnosticMap.get(diagDocument);
871+
compilerAssert(
872+
diagnostics,
873+
"Diagnostic reported against a source file that was not added to the program.",
874+
);
875+
diagnostics.push(diagnostic);
876+
newDiagnosticIndex.set(diagnostic.data.id, unusedSuppressionDiagnostic);
877+
}
878+
}
879+
847880
// Atomically swap the diagnostic index so that in-flight code action resolves
848881
// referencing old diagnostic IDs can still find their entries until new diagnostics are sent.
849882
currentDiagnosticIndex = newDiagnosticIndex;

0 commit comments

Comments
 (0)