From 64be7432ce70d4d17f9d4dfc9b745e967f493510 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 13 Dec 2024 13:52:53 +0800 Subject: [PATCH 01/22] add code for unused using --- .../remove-unused-code.codefix.ts | 16 + packages/compiler/src/core/diagnostics.ts | 4 +- packages/compiler/src/core/messages.ts | 6 + packages/compiler/src/core/name-resolver.ts | 49 +- packages/compiler/src/core/program.ts | 28 + packages/compiler/src/core/types.ts | 5 +- packages/compiler/src/server/diagnostics.ts | 4 +- packages/compiler/src/server/serverlib.ts | 5 + packages/compiler/src/testing/expect.ts | 2 +- .../test/checker/unused-using.test.ts | 852 ++++++++++++++++++ 10 files changed, 964 insertions(+), 7 deletions(-) create mode 100644 packages/compiler/src/core/compiler-code-fixes/remove-unused-code.codefix.ts create mode 100644 packages/compiler/test/checker/unused-using.test.ts diff --git a/packages/compiler/src/core/compiler-code-fixes/remove-unused-code.codefix.ts b/packages/compiler/src/core/compiler-code-fixes/remove-unused-code.codefix.ts new file mode 100644 index 00000000000..8fa20b4feab --- /dev/null +++ b/packages/compiler/src/core/compiler-code-fixes/remove-unused-code.codefix.ts @@ -0,0 +1,16 @@ +import { defineCodeFix, getSourceLocation } from "../diagnostics.js"; +import { type ImportStatementNode, type UsingStatementNode } from "../types.js"; + +/** + * Quick fix that remove unused code. + */ +export function removeUnusedCodeCodeFix(node: ImportStatementNode | UsingStatementNode) { + return defineCodeFix({ + id: "remove-unused-code", + label: `Remove unused code`, + fix: (context) => { + const location = getSourceLocation(node); + return context.replaceText(location, ""); + }, + }); +} diff --git a/packages/compiler/src/core/diagnostics.ts b/packages/compiler/src/core/diagnostics.ts index f234a975127..eb4033ae229 100644 --- a/packages/compiler/src/core/diagnostics.ts +++ b/packages/compiler/src/core/diagnostics.ts @@ -33,7 +33,7 @@ export type DiagnosticHandler = (diagnostic: Diagnostic) => void; export function logDiagnostics(diagnostics: readonly Diagnostic[], logger: LogSink) { for (const diagnostic of diagnostics) { logger.log({ - level: diagnostic.severity, + level: diagnostic.severity === "hint" ? "trace" : diagnostic.severity, message: diagnostic.message, code: diagnostic.code, url: diagnostic.url, @@ -52,7 +52,7 @@ export function formatDiagnostic(diagnostic: Diagnostic, options: FormatDiagnost return formatLog( { code: diagnostic.code, - level: diagnostic.severity, + level: diagnostic.severity === "hint" ? "trace" : diagnostic.severity, message: diagnostic.message, url: diagnostic.url, sourceLocation: getSourceLocation(diagnostic.target, { locateId: true }), diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index a0b74d2d583..a999b25528d 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -576,6 +576,12 @@ const diagnostics = { default: "The #deprecated directive cannot be used more than once on the same declaration.", }, }, + "unused-using": { + severity: "hint", + messages: { + default: paramMessage`Unused using: ${"code"}`, + }, + }, /** * Configuration diff --git a/packages/compiler/src/core/name-resolver.ts b/packages/compiler/src/core/name-resolver.ts index 3641bdb78be..7057c220cd7 100644 --- a/packages/compiler/src/core/name-resolver.ts +++ b/packages/compiler/src/core/name-resolver.ts @@ -60,7 +60,7 @@ import { Mutable, mutate } from "../utils/misc.js"; import { createSymbol, createSymbolTable, getSymNode } from "./binder.js"; import { compilerAssert } from "./diagnostics.js"; -import { visitChildren } from "./parser.js"; +import { getFirstAncestor, visitChildren } from "./parser.js"; import { Program } from "./program.js"; import { AliasStatementNode, @@ -94,6 +94,7 @@ import { TypeReferenceNode, TypeSpecScriptNode, UnionStatementNode, + UsingStatementNode, } from "./types.js"; export interface NameResolver { @@ -142,6 +143,9 @@ export interface NameResolver { node: TypeReferenceNode | IdentifierNode | MemberExpressionNode, ): ResolutionResult; + /** Get the using statement nodes which is not used in resolving yet */ + getUnusedUsings(): UsingStatementNode[]; + /** Built-in symbols. */ readonly symbols: { /** Symbol for the global namespace */ @@ -175,6 +179,11 @@ export function createResolver(program: Program): NameResolver { mutate(globalNamespaceNode).symbol = globalNamespaceSym; mutate(globalNamespaceSym.exports).set(globalNamespaceNode.id.sv, globalNamespaceSym); + /** + * Tracking the symbols that are used through using. + */ + const usedUsingSym = new Map>(); + const metaTypePrototypes = createMetaTypePrototypes(); const nullSym = createSymbol(undefined, "null", SymbolFlags.None); @@ -222,8 +231,33 @@ export function createResolver(program: Program): NameResolver { resolveTypeReference, getAugmentDecoratorsForSym, + getUnusedUsings, }; + function getUnusedUsings(): UsingStatementNode[] { + const unusedUsings: Set = new Set(); + for (const file of program.sourceFiles.values()) { + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type === "project") { + const usedSym = usedUsingSym.get(file) ?? new Set(); + for (const using of file.usings) { + const table = getNodeLinks(using.name).resolvedSymbol; + let used = false; + for (const [_, sym] of table?.exports ?? new Map()) { + if (usedSym.has(getMergedSymbol(sym))) { + used = true; + break; + } + } + if (used === false) { + unusedUsings.add(using); + } + } + } + } + return [...unusedUsings]; + } + function getAugmentDecoratorsForSym(sym: Sym) { return augmentDecoratorsForSym.get(sym) ?? []; } @@ -960,6 +994,15 @@ export function createResolver(program: Program): NameResolver { if ("locals" in scope && scope.locals !== undefined) { binding = tableLookup(scope.locals, node, options.resolveDecorators); if (binding) { + if (binding.flags & SymbolFlags.Using && binding.symbolSource) { + const fileNode = getFirstAncestor(node, (n) => n.kind === SyntaxKind.TypeSpecScript) as + | TypeSpecScriptNode + | undefined; + if (fileNode) { + usedUsingSym.get(fileNode)?.add(binding.symbolSource) ?? + usedUsingSym.set(fileNode, new Set([binding.symbolSource])); + } + } return resolvedResult(binding); } } @@ -997,6 +1040,10 @@ export function createResolver(program: Program): NameResolver { []), ]); } + if (usingBinding.flags & SymbolFlags.Using && usingBinding.symbolSource) { + usedUsingSym.get(scope)?.add(usingBinding.symbolSource) ?? + usedUsingSym.set(scope, new Set([usingBinding.symbolSource])); + } return resolvedResult(usingBinding.symbolSource!); } } diff --git a/packages/compiler/src/core/program.ts b/packages/compiler/src/core/program.ts index ff73599e6d6..980a3b2ad23 100644 --- a/packages/compiler/src/core/program.ts +++ b/packages/compiler/src/core/program.ts @@ -15,6 +15,7 @@ import { PackageJson } from "../types/package-json.js"; import { deepEquals, findProjectRoot, isDefined, mapEquals, mutate } from "../utils/misc.js"; import { createBinder } from "./binder.js"; import { Checker, createChecker } from "./checker.js"; +import { removeUnusedCodeCodeFix } from "./compiler-code-fixes/remove-unused-code.codefix.js"; import { createSuppressCodeFix } from "./compiler-code-fixes/suppress.codefix.js"; import { compilerAssert } from "./diagnostics.js"; import { resolveTypeSpecEntrypoint } from "./entrypoint-resolution.js"; @@ -45,11 +46,13 @@ import { EmitContext, EmitterFunc, Entity, + IdentifierNode, JsSourceFileNode, LibraryInstance, LibraryMetadata, LiteralType, LocationContext, + MemberExpressionNode, ModuleLibraryMetadata, Namespace, NoTarget, @@ -250,6 +253,8 @@ export async function compile( return program; } + validateUnusedCode(); + // Linter stage program.reportDiagnostics(linter.lint()); @@ -260,6 +265,29 @@ export async function compile( return program; + function validateUnusedCode() { + const getUsingName = (node: MemberExpressionNode | IdentifierNode): string => { + if (node.kind === SyntaxKind.MemberExpression) { + return `${getUsingName(node.base)}${node.selector}${node.id.sv}`; + } else { + // identifier node + return node.sv; + } + }; + resolver.getUnusedUsings().forEach((target) => { + reportDiagnostic( + createDiagnostic({ + code: "unused-using", + target: target, + format: { + code: `using ${getUsingName(target.name)}`, + }, + codefixes: [removeUnusedCodeCodeFix(target)], + }), + ); + }); + } + /** * Validate the libraries loaded during the compilation process are compatible. */ diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index ec318179682..93803cbdb7b 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -2300,7 +2300,7 @@ export interface TemplateInstanceTarget { export type DiagnosticTarget = TypeSpecDiagnosticTarget | SourceLocation; -export type DiagnosticSeverity = "error" | "warning"; +export type DiagnosticSeverity = "error" | "warning" | "hint"; export interface Diagnostic { code: string; @@ -2529,8 +2529,9 @@ export interface DiagnosticDefinition { * Diagnostic severity. * - `warning` - Suppressable, should be used to represent potential issues but not blocking. * - `error` - Non-suppressable, should be used to represent failure to move forward. + * - `hint` - Something to hint to a better way of doing it, like proposing a refactoring. */ - readonly severity: "warning" | "error"; + readonly severity: "warning" | "error" | "hint"; /** Messages that can be reported with the diagnostic. */ readonly messages: M; /** Short description of the diagnostic */ diff --git a/packages/compiler/src/server/diagnostics.ts b/packages/compiler/src/server/diagnostics.ts index f9dbe4718a2..5c5f6876767 100644 --- a/packages/compiler/src/server/diagnostics.ts +++ b/packages/compiler/src/server/diagnostics.ts @@ -141,11 +141,13 @@ function getVSLocationWithTypeInfo( }; } -function convertSeverity(severity: "warning" | "error"): DiagnosticSeverity { +function convertSeverity(severity: "warning" | "error" | "hint"): DiagnosticSeverity { switch (severity) { case "warning": return DiagnosticSeverity.Warning; case "error": return DiagnosticSeverity.Error; + case "hint": + return DiagnosticSeverity.Hint; } } diff --git a/packages/compiler/src/server/serverlib.ts b/packages/compiler/src/server/serverlib.ts index eb47f25d095..80f3f037a8a 100644 --- a/packages/compiler/src/server/serverlib.ts +++ b/packages/compiler/src/server/serverlib.ts @@ -405,6 +405,11 @@ export function createServer(host: ServerHost): Server { } if (each.code === "deprecated") { diagnostic.tags = [DiagnosticTag.Deprecated]; + } else if (each.code === "unused-using") { + // Unused or unnecessary code. Diagnostics with this tag are rendered faded out, so no extra work needed from IDE side + // https://vscode-api.js.org/enums/vscode.DiagnosticTag.html#google_vignette + // https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.languageserver.protocol.diagnostictag?view=visualstudiosdk-2022 + diagnostic.tags = [DiagnosticTag.Unnecessary]; } diagnostic.data = { id: diagnosticIdCounter++ }; const diagnostics = diagnosticMap.get(diagDocument); diff --git a/packages/compiler/src/testing/expect.ts b/packages/compiler/src/testing/expect.ts index 4732daddf33..c710c6ca422 100644 --- a/packages/compiler/src/testing/expect.ts +++ b/packages/compiler/src/testing/expect.ts @@ -33,7 +33,7 @@ export interface DiagnosticMatch { /** * Match the severity. */ - severity?: "error" | "warning"; + severity?: "error" | "warning" | "hint"; /** * Name of the file for this diagnostic. diff --git a/packages/compiler/test/checker/unused-using.test.ts b/packages/compiler/test/checker/unused-using.test.ts new file mode 100644 index 00000000000..15cf3541905 --- /dev/null +++ b/packages/compiler/test/checker/unused-using.test.ts @@ -0,0 +1,852 @@ +import { beforeEach, describe, it } from "vitest"; +import { + TestHost, + createTestHost, + expectDiagnosticEmpty, + expectDiagnostics, +} from "../../src/testing/index.js"; + +describe("compiler: unused using statements", () => { + let testHost: TestHost; + + beforeEach(async () => { + testHost = await createTestHost(); + }); + + it("no unused diagnostic when using is used", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + + using N; + using M; + model Z { a: X, b: Y} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + namespace M; + model Y { y: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnosticEmpty(diagnostics); + }); + + it("report for unused using", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + + model Z { a: N.X, b: Y} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + using N; + model Y { y: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + ]); + }); + + it("report for same unused using from different file", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + + using N; + model Z { a: Y, b: Y} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + using N; + model Y { y: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + ]); + }); + + it("report for multiple unused using in one file", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + + using N; + using M; + model Z { a: N.X, b: M.Y} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + namespace M; + model Y { y: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using M", + severity: "hint", + }, + ]); + }); + + it("report for unused using when there is used using", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + + using N; + using M; + model Z { a: X, b: M.Y} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + namespace M; + model Y { y: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using M", + severity: "hint", + }, + ]); + }); + + it("using in namespaces", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + namespace N{ + model X { x: int32 } + } + namespace M{ + model XX {xx: Z.Y } + } + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace Z; + using N; + using M; + @test model Y { ... X } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using M", + severity: "hint", + }, + ]); + }); + + it("using in the same file", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + namespace N { + using M; + model X { x: XX } + } + namespace M { + using N; + model XX {xx: N.X } + } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + ]); + }); + + it("works with dotted namespaces", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + using N.M; + namespace Z { + alias test = Y; + } + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.M; + model X { x: int32 } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + using N.M; + @test model Y { ...N.M.X } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N.M", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using N.M", + severity: "hint", + }, + ]); + }); + + it("TypeSpec.Xyz namespace doesn't need TypeSpec prefix in using", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + import "./c.tsp"; + using TypeSpec.Xyz; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace TypeSpec.Xyz; + model X { x: Y } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + using Xyz; + @test model Y { ... Xyz.X, ... Z } + `, + ); + testHost.addTypeSpecFile( + "c.tsp", + ` + using Xyz; + @test model Z { ... X } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using TypeSpec.Xyz", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Xyz", + severity: "hint", + }, + ]); + }); + + it("2 namespace with the same last name", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + using N.A; + using M.A; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.A { + model B { } + } + namespace M.A { + model B { } + } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N.A", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using M.A", + severity: "hint", + }, + ]); + }); + + it("one namespace from two file, no unused using when just refering to one of them", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + using N.M; + model Z { b: B2} + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.M { + model B2 { } + } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + namespace N.M { + model B { } + } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnosticEmpty(diagnostics); + }); + + it("one namespace from two file, show unused using when none is referred", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + using N.M; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.M { + model B2 { } + } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + namespace N.M { + model B { } + } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N.M", + severity: "hint", + }, + ]); + }); + + it("unused invalid using, no unnecessary diagnostic when there is other error", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + using N.M2; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.M; + model X { x: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "invalid-ref", + severity: "error", + }, + ]); + }); + + it("unused using along with duplicate usings, no unnecessary diagnostic when there is other error", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + using N.M; + using N.M; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N.M; + model X { x: int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "duplicate-using", + message: 'duplicate using of "N.M" namespace', + }, + { + code: "duplicate-using", + message: 'duplicate using of "N.M" namespace', + }, + ]); + }); + + it("does not throws errors for different usings with the same bindings if not used", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + + namespace Ns { + using N; + namespace Ns2 { + using M; + namespace Ns3 { + using L; + alias a2 = A2; + } + } + alias a = A3; + } + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N { + model A1 { } + } + namespace M { + model A { } + } + namespace L { + model A2 { } + } + namespace Ns.N { + model A3 { } + } + `, + ); + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using M", + severity: "hint", + }, + ]); + }); + + it("using multi-level namespace", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + import "./c.tsp"; + import "./d.tsp"; + + namespace Ns1 { + model A1 { } + namespace Ns2 { + model A2 { } + namespace Ns3 { + model A3 { } + } + } + } + model Test { + a: A; + b: B; + c: C; + d: D; + } + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + using Ns1; + using Ns1.Ns2; + using Ns1.Ns2.Ns3; + model A { } + `, + ); + testHost.addTypeSpecFile( + "b.tsp", + ` + using Ns1; + using Ns1.Ns2; + using Ns1.Ns2.Ns3; + model B { a: A1 } + `, + ); + testHost.addTypeSpecFile( + "c.tsp", + ` + using Ns1; + using Ns1.Ns2; + using Ns1.Ns2.Ns3; + model C { a: A2 } + `, + ); + testHost.addTypeSpecFile( + "d.tsp", + ` + using Ns1; + using Ns1.Ns2; + using Ns1.Ns2.Ns3; + model D { a: A3 } + `, + ); + const diagnostics = await testHost.diagnose("./"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using Ns1", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2.Ns3", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2.Ns3", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2.Ns3", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using Ns1.Ns2", + severity: "hint", + }, + ]); + }); + + it("no report unused using when the ref is ambiguous (error) while others not impacted", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + using N; + using M; + model B extends A {}; + model B2 extends C {}; + `, + ); + testHost.addTypeSpecFile( + "a.tsp", + ` + namespace N { + model A { } + } + namespace M { + model A { } + model C { } + } + `, + ); + + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: + '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', + }, + ]); + }); + + it("no not-used using for decorator", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./doc.js"; + namespace Test; + using A; + @dec1 + namespace Foo {} + `, + ); + + testHost.addJsFile("doc.js", { + namespace: "Test.A", + $dec1() {}, + }); + + const diagnostics = await testHost.diagnose("./"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused using for TypeSpec", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + namespace Foo; + using TypeSpec; + model Bar { a : TypeSpec.int32 } + `, + ); + + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using TypeSpec", + severity: "hint", + }, + ]); + }); + + it("works same name in different namespace", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./other.tsp"; + namespace Main { + using Other; + model OtherModel { + } + model MainModel { + a: OtherModel; + } + } + `, + ); + testHost.addTypeSpecFile( + "other.tsp", + ` + namespace Other { + model OtherModel { + } + } + `, + ); + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using Other", + severity: "hint", + }, + ]); + }); + + it("not used using for lib", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + using LibNs; + model A { x: int16; } + `, + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + import "./lib-a.tsp"; + namespace LibNs { + model LibMainModel{ } + } + `, + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/lib-a.tsp", + ` + namespace LibNs; + model LibAModel { } + `, + ); + + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using LibNs", + severity: "hint", + }, + ]); + }); + + it("no not-used using for lib", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + using LibNs; + model A { x: LibAModel; } + `, + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + import "./lib-a.tsp"; + namespace LibNs { + model LibMainModel{ } + } + `, + ); + testHost.addTypeSpecFile( + "node_modules/my-lib/lib-a.tsp", + ` + namespace LibNs; + model LibAModel { } + `, + ); + + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnosticEmpty(diagnostics); + }); +}); From a1862d84bc4319ee7e1ac5d797573210cb11412d Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 13 Dec 2024 14:13:25 +0800 Subject: [PATCH 02/22] fix some tests --- .../compiler/test/checker/decorators.test.ts | 130 ++++++++----- packages/compiler/test/checker/using.test.ts | 180 ++++++++++++------ 2 files changed, 212 insertions(+), 98 deletions(-) diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts index 41e05c83f18..72e6ff1128a 100644 --- a/packages/compiler/test/checker/decorators.test.ts +++ b/packages/compiler/test/checker/decorators.test.ts @@ -11,6 +11,7 @@ import { numericRanges } from "../../src/core/numeric-ranges.js"; import { Numeric } from "../../src/core/numeric.js"; import { BasicTestRunner, + DiagnosticMatch, TestHost, createTestHost, createTestWrapper, @@ -42,6 +43,18 @@ describe("compiler: checker: decorators", () => { }); }); + const unnecessaryDiags: DiagnosticMatch[] = [ + { + code: "unused-using", + message: `Unused using: using TypeSpec.Reflection`, + severity: "hint", + }, + ]; + const compileAndCheckDiags = async (code: string) => { + const [_, diags] = await runner.compileAndDiagnose(code); + expectDiagnostics(diags, [...unnecessaryDiags]); + }; + describe("bind implementation to declaration", () => { let $otherDec: DecoratorFunction; function expectDecorator(ns: Namespace) { @@ -57,7 +70,7 @@ describe("compiler: checker: decorators", () => { }); it("defined at root", async () => { - await runner.compile(` + await compileAndCheckDiags(` extern dec otherDec(target: unknown); `); @@ -67,7 +80,7 @@ describe("compiler: checker: decorators", () => { it("in a namespace", async () => { setTypeSpecNamespace("Foo.Bar", $otherDec); - await runner.compile(` + await compileAndCheckDiags(` namespace Foo.Bar { extern dec otherDec(target: unknown); } @@ -86,7 +99,7 @@ describe("compiler: checker: decorators", () => { it("defined at root", async () => { testJs.$decorators = { "": { otherDec: $otherDec } }; - await runner.compile(` + await compileAndCheckDiags(` extern dec otherDec(target: unknown); `); @@ -96,7 +109,7 @@ describe("compiler: checker: decorators", () => { it("in a namespace", async () => { testJs.$decorators = { "Foo.Bar": { otherDec: $otherDec } }; - await runner.compile(` + await compileAndCheckDiags(` namespace Foo.Bar { extern dec otherDec(target: unknown); } @@ -116,30 +129,36 @@ describe("compiler: checker: decorators", () => { const diagnostics = await runner.diagnose(` dec testDec(target: unknown); `); - expectDiagnostics(diagnostics, { - code: "decorator-extern", - message: "A decorator declaration must be prefixed with the 'extern' modifier.", - }); + expectDiagnostics(diagnostics, [ + { + code: "decorator-extern", + message: "A decorator declaration must be prefixed with the 'extern' modifier.", + }, + ]); }); it("errors if rest parameter type is not an array expression", async () => { const diagnostics = await runner.diagnose(` extern dec testDec(target: unknown, ...rest: string); `); - expectDiagnostics(diagnostics, { - code: "rest-parameter-array", - message: "A rest parameter must be of an array type.", - }); + expectDiagnostics(diagnostics, [ + { + code: "rest-parameter-array", + message: "A rest parameter must be of an array type.", + }, + ]); }); it("errors if extern decorator is missing implementation", async () => { const diagnostics = await runner.diagnose(` extern dec notImplemented(target: unknown); `); - expectDiagnostics(diagnostics, { - code: "missing-implementation", - message: "Extern declaration must have an implementation in JS file.", - }); + expectDiagnostics(diagnostics, [ + { + code: "missing-implementation", + message: "Extern declaration must have an implementation in JS file.", + }, + ]); }); }); @@ -160,6 +179,19 @@ describe("compiler: checker: decorators", () => { }); }); + const unnecessaryDiags: DiagnosticMatch[] = [ + { + code: "unused-using", + message: `Unused using: using TypeSpec.Reflection`, + severity: "hint", + }, + ]; + const compileAndCheckDiags = async (code: string) => { + const [r, diags] = await runner.compileAndDiagnose(code); + expectDiagnostics(diags, [...unnecessaryDiags]); + return r; + }; + function expectDecoratorCalledWith(target: unknown, ...args: unknown[]) { ok(calledArgs, "Decorator was not called."); strictEqual(calledArgs.length, 2 + args.length); @@ -175,7 +207,7 @@ describe("compiler: checker: decorators", () => { } it("calls a decorator with no argument", async () => { - const { Foo } = await runner.compile(` + const { Foo } = await compileAndCheckDiags(` extern dec testDec(target: unknown); @testDec @@ -187,7 +219,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with arguments", async () => { - const { Foo } = await runner.compile(` + const { Foo } = await compileAndCheckDiags(` extern dec testDec(target: unknown, arg1: valueof string, arg2: valueof string); @testDec("one", "two") @@ -199,7 +231,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with optional arguments", async () => { - const { Foo } = await runner.compile(` + const { Foo } = await compileAndCheckDiags(` extern dec testDec(target: unknown, arg1: valueof string, arg2?: valueof string); @testDec("one") @@ -211,7 +243,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with rest arguments", async () => { - const { Foo } = await runner.compile(` + const { Foo } = await compileAndCheckDiags(` extern dec testDec(target: unknown, arg1: valueof string, ...args: valueof string[]); @testDec("one", "two", "three", "four") @@ -230,10 +262,12 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, { - code: "invalid-argument-count", - message: "Expected 2 arguments, but got 1.", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument-count", + message: "Expected 2 arguments, but got 1.", + }, + ]); expectDecoratorNotCalled(); }); @@ -245,10 +279,12 @@ describe("compiler: checker: decorators", () => { @test model Foo {} `); - expectDiagnostics(diagnostics, { - code: "invalid-argument-count", - message: "Expected 1-2 arguments, but got 3.", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument-count", + message: "Expected 1-2 arguments, but got 3.", + }, + ]); expectDecoratorCalledWith(Foo, "one", "two"); }); @@ -260,10 +296,12 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, { - code: "invalid-argument-count", - message: "Expected 0 arguments, but got 1.", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument-count", + message: "Expected 0 arguments, but got 1.", + }, + ]); }); it("errors if not calling with too few arguments with rest", async () => { @@ -274,10 +312,12 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, { - code: "invalid-argument-count", - message: "Expected at least 1 arguments, but got 0.", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument-count", + message: "Expected at least 1 arguments, but got 0.", + }, + ]); expectDecoratorNotCalled(); }); @@ -304,10 +344,12 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, { - code: "invalid-argument", - message: "Argument of type '123' is not assignable to parameter of type 'string'", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument", + message: "Argument of type '123' is not assignable to parameter of type 'string'", + }, + ]); expectDecoratorNotCalled(); }); @@ -334,7 +376,7 @@ describe("compiler: checker: decorators", () => { // Regresssion test for https://github.com/microsoft/typespec/issues/3211 it("augmenting a template model property before a decorator declaration resolve the declaration correctly", async () => { - await runner.compile(` + await compileAndCheckDiags(` model Foo { prop: T; } @@ -353,7 +395,7 @@ describe("compiler: checker: decorators", () => { value: string, suppress?: boolean, ): Promise { - await runner.compile(` + await compileAndCheckDiags(` extern dec testDec(target: unknown, arg1: ${type}); ${suppress ? `#suppress "deprecated" "for testing"` : ""} @@ -527,7 +569,9 @@ describe("compiler: checker: decorators", () => { @test model Foo {} `); - expectDiagnosticEmpty(diagnostics.filter((x) => x.code !== "deprecated")); + expectDiagnosticEmpty( + diagnostics.filter((x) => x.code !== "deprecated" && x.code !== "unused-using"), + ); return calledArgs![2]; } diff --git a/packages/compiler/test/checker/using.test.ts b/packages/compiler/test/checker/using.test.ts index 736e4b5052e..13aac35a4e4 100644 --- a/packages/compiler/test/checker/using.test.ts +++ b/packages/compiler/test/checker/using.test.ts @@ -1,12 +1,7 @@ import { rejects, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { Model } from "../../src/core/types.js"; -import { - TestHost, - createTestHost, - expectDiagnosticEmpty, - expectDiagnostics, -} from "../../src/testing/index.js"; +import { TestHost, createTestHost, expectDiagnostics } from "../../src/testing/index.js"; describe("compiler: using statements", () => { let testHost: TestHost; @@ -21,6 +16,7 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; + alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -51,6 +47,7 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; + alias foo = Z.Y; `, ); testHost.addTypeSpecFile( @@ -82,6 +79,7 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; + alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -125,8 +123,14 @@ describe("compiler: using statements", () => { `, ); testHost.addTypeSpecFile("b.tsp", `namespace B { model BModel {} }`); - - expectDiagnosticEmpty(await testHost.diagnose("./")); + const diags = await testHost.diagnose("./"); + expectDiagnostics(diags, [ + { + code: "unused-using", + message: "Unused using: using A", + severity: "hint", + }, + ]); }); it("TypeSpec.Xyz namespace doesn't need TypeSpec prefix in using", async () => { @@ -135,6 +139,7 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; + alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -189,7 +194,18 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnosticEmpty(diagnostics); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N.A", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using M.A", + severity: "hint", + }, + ]); }); describe("duplicate usings", () => { @@ -211,7 +227,23 @@ describe("compiler: using statements", () => { testHost.addTypeSpecFile("a.tsp", `namespace A { model AModel {} }`); const diagnostics = await testHost.diagnose("./"); - expectDiagnosticEmpty(diagnostics); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using A", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using A", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using A", + severity: "hint", + }, + ]); }); it("throws errors for duplicate imported usings", async () => { @@ -276,7 +308,18 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnosticEmpty(diagnostics); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using M", + severity: "hint", + }, + ]); }); it("report ambiguous diagnostics when using name present in multiple using", async () => { @@ -310,13 +353,16 @@ describe("compiler: using statements", () => { `, ); const diagnostics = await testHost.diagnose("./", { nostdlib: true }); - expectDiagnostics(diagnostics, [ - { - code: "ambiguous-symbol", - message: - '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', - }, - ]); + expectDiagnostics( + diagnostics.filter((d) => d.code !== "unused-using"), + [ + { + code: "ambiguous-symbol", + message: + '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', + }, + ], + ); }); it("report ambiguous diagnostics when symbol exists in using namespace and global namespace", async () => { @@ -347,13 +393,16 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./", { nostdlib: true }); - expectDiagnostics(diagnostics, [ - { - code: "ambiguous-symbol", - message: - '"M" is an ambiguous name between global.M, B.M. Try using fully qualified name instead: global.M, B.M', - }, - ]); + expectDiagnostics( + diagnostics.filter((d) => d.code !== "unused-using"), + [ + { + code: "ambiguous-symbol", + message: + '"M" is an ambiguous name between global.M, B.M. Try using fully qualified name instead: global.M, B.M', + }, + ], + ); }); it("reports ambiguous symbol for decorator", async () => { @@ -380,12 +429,15 @@ describe("compiler: using statements", () => { }); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "ambiguous-symbol", - message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, - }, - ]); + expectDiagnostics( + diagnostics.filter((d) => d.code !== "unused-using"), + [ + { + code: "ambiguous-symbol", + message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, + }, + ], + ); }); it("reports ambiguous symbol for decorator with missing implementation", async () => { @@ -406,13 +458,16 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "ambiguous-symbol", - message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, - }, - { code: "missing-implementation" }, - ]); + expectDiagnostics( + diagnostics.filter((d) => d.code !== "unused-using"), + [ + { + code: "ambiguous-symbol", + message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, + }, + { code: "missing-implementation" }, + ], + ); }); it("ambiguous use doesn't affect other files", async () => { @@ -456,14 +511,17 @@ describe("compiler: using statements", () => { `, ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "ambiguous-symbol", - message: - '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', - file: /ambiguous\.tsp$/, - }, - ]); + expectDiagnostics( + diagnostics.filter((d) => d.code !== "unused-using"), + [ + { + code: "ambiguous-symbol", + message: + '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', + file: /ambiguous\.tsp$/, + }, + ], + ); }); it("resolves 'local' decls over usings", async () => { @@ -493,11 +551,19 @@ describe("compiler: using statements", () => { `, ); - const { B } = (await testHost.compile("./")) as { + const [result, diags] = await testHost.compileAndDiagnose("./"); + const { B } = result as { B: Model; }; strictEqual(B.properties.size, 1); strictEqual(B.properties.get("a")!.type.kind, "Union"); + expectDiagnostics(diags, [ + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + ]); }); it("usings are local to a file", async () => { @@ -585,10 +651,12 @@ describe("emit diagnostics", () => { const diagnostics = await diagnose(` using NotDefined; `); - expectDiagnostics(diagnostics, { - code: "invalid-ref", - message: "Unknown identifier NotDefined", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-ref", + message: "Unknown identifier NotDefined", + }, + ]); }); describe("when using non-namespace types", () => { @@ -605,10 +673,12 @@ describe("emit diagnostics", () => { using Target; ${code} `); - expectDiagnostics(diagnostics, { - code: "using-invalid-ref", - message: "Using must refer to a namespace", - }); + expectDiagnostics(diagnostics, [ + { + code: "using-invalid-ref", + message: "Using must refer to a namespace", + }, + ]); }); }); }); From bf0c11bcca7631accd1b99e1af2522a753d3e7fc Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 26 Dec 2024 19:12:54 +0800 Subject: [PATCH 03/22] add changelog --- .chronus/changes/unused-using-2024-11-26-19-12-40.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/unused-using-2024-11-26-19-12-40.md diff --git a/.chronus/changes/unused-using-2024-11-26-19-12-40.md b/.chronus/changes/unused-using-2024-11-26-19-12-40.md new file mode 100644 index 00000000000..bc220259186 --- /dev/null +++ b/.chronus/changes/unused-using-2024-11-26-19-12-40.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Support diagnostics for unused using statement \ No newline at end of file From a01cd7febed04c8092fb770929d30cfc2097925f Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 26 Dec 2024 19:22:47 +0800 Subject: [PATCH 04/22] report hint as info to server log --- packages/compiler/src/server/serverlib.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/src/server/serverlib.ts b/packages/compiler/src/server/serverlib.ts index f81dd10c0c7..74bc95afbdb 100644 --- a/packages/compiler/src/server/serverlib.ts +++ b/packages/compiler/src/server/serverlib.ts @@ -317,7 +317,7 @@ export function createServer(host: ServerHost): Server { if (!validationResult.valid) { for (const diag of validationResult.diagnostics) { log({ - level: diag.severity, + level: diag.severity === "hint" ? "info" : diag.severity, message: diag.message, detail: { code: diag.code, From d50e9590c4c737fe746a6de146029c2b2a16dc24 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 10:23:32 +0800 Subject: [PATCH 05/22] fix tests --- packages/compiler/src/testing/test-host.ts | 16 +++++++++++----- packages/compiler/src/testing/types.ts | 2 ++ packages/http/test/test-host.ts | 1 + packages/openapi3/test/test-host.ts | 1 + .../test/tsp-openapi3/utils/tsp-for-openapi3.ts | 1 + 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/testing/test-host.ts b/packages/compiler/src/testing/test-host.ts index 9b38cf08673..4d31bb841b9 100644 --- a/packages/compiler/src/testing/test-host.ts +++ b/packages/compiler/src/testing/test-host.ts @@ -242,7 +242,7 @@ export const StandardTestLibrary: TypeSpecTestLibrary = { }; export async function createTestHost(config: TestHostConfig = {}): Promise { - const testHost = await createTestHostInternal(); + const testHost = await createTestHostInternal(config); await testHost.addTypeSpecLibrary(StandardTestLibrary); if (config.libraries) { for (const library of config.libraries) { @@ -257,7 +257,7 @@ export async function createTestRunner(host?: TestHost): Promise { +async function createTestHostInternal(config: TestHostConfig): Promise { let program: Program | undefined; const libraries: TypeSpecTestLibrary[] = []; const testTypes: Record = {}; @@ -315,7 +315,10 @@ async function createTestHostInternal(): Promise { async function compile(main: string, options: CompilerOptions = {}) { const [testTypes, diagnostics] = await compileAndDiagnose(main, options); - expectDiagnosticEmpty(diagnostics); + const filteredDiagnostics = config.diagnosticFilter + ? diagnostics.filter(config.diagnosticFilter) + : diagnostics; + expectDiagnosticEmpty(filteredDiagnostics); return testTypes; } @@ -334,10 +337,13 @@ async function createTestHostInternal(): Promise { } const p = await compileProgram(fileSystem.compilerHost, resolveVirtualPath(mainFile), options); program = p; + const filteredDiagnostics = config.diagnosticFilter + ? p.diagnostics.filter(config.diagnosticFilter) + : p.diagnostics; logVerboseTestOutput((log) => - logDiagnostics(p.diagnostics, createLogger({ sink: fileSystem.compilerHost.logSink })), + logDiagnostics(filteredDiagnostics, createLogger({ sink: fileSystem.compilerHost.logSink })), ); - return [testTypes, p.diagnostics]; + return [testTypes, filteredDiagnostics]; } } diff --git a/packages/compiler/src/testing/types.ts b/packages/compiler/src/testing/types.ts index 833c0520974..99204b60608 100644 --- a/packages/compiler/src/testing/types.ts +++ b/packages/compiler/src/testing/types.ts @@ -54,6 +54,8 @@ export interface TypeSpecTestLibrary { export interface TestHostConfig { libraries?: TypeSpecTestLibrary[]; + /** the diagnostics will be ignored if this return false */ + diagnosticFilter?: (diag: Diagnostic) => boolean; } export class TestHostError extends Error { diff --git a/packages/http/test/test-host.ts b/packages/http/test/test-host.ts index e3306f1eefa..71c8be3d526 100644 --- a/packages/http/test/test-host.ts +++ b/packages/http/test/test-host.ts @@ -18,6 +18,7 @@ import { HttpTestLibrary } from "../src/testing/index.js"; export async function createHttpTestHost(): Promise { return createTestHost({ libraries: [HttpTestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createHttpTestRunner(): Promise { diff --git a/packages/openapi3/test/test-host.ts b/packages/openapi3/test/test-host.ts index 2be1f42e0fd..5538f50ff62 100644 --- a/packages/openapi3/test/test-host.ts +++ b/packages/openapi3/test/test-host.ts @@ -25,6 +25,7 @@ export async function createOpenAPITestHost() { OpenAPITestLibrary, OpenAPI3TestLibrary, ], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } diff --git a/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts b/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts index f746fe549e1..43c70313370 100644 --- a/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts +++ b/packages/openapi3/test/tsp-openapi3/utils/tsp-for-openapi3.ts @@ -57,6 +57,7 @@ export async function compileForOpenAPI3({ parameters, paths, schemas }: OpenAPI const testableCode = wrapCodeInTest(code); const host = await createTestHost({ libraries: [HttpTestLibrary, OpenAPITestLibrary, OpenAPI3TestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); host.addTypeSpecFile("main.tsp", testableCode); From 04781874abc3b87c4dbce65c4343e36a9b871046 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 11:33:18 +0800 Subject: [PATCH 06/22] fix test --- packages/http-server-csharp/test/test-host.ts | 1 + packages/rest/test/test-host.ts | 1 + packages/samples/specs/versioning/main.tsp | 1 - packages/samples/specs/visibility/visibility.tsp | 1 - packages/xml/test/test-host.ts | 1 + 5 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/http-server-csharp/test/test-host.ts b/packages/http-server-csharp/test/test-host.ts index 92ccc011681..31fdf67f7b5 100644 --- a/packages/http-server-csharp/test/test-host.ts +++ b/packages/http-server-csharp/test/test-host.ts @@ -13,6 +13,7 @@ export async function createCSharpServiceEmitterTestHost() { VersioningTestLibrary, CSharpServiceEmitterTestLibrary, ], + diagnosticFilter: (diag) => diag.severity !== "hint", }); return result; diff --git a/packages/rest/test/test-host.ts b/packages/rest/test/test-host.ts index 564e0485068..c9e19239cf7 100644 --- a/packages/rest/test/test-host.ts +++ b/packages/rest/test/test-host.ts @@ -19,6 +19,7 @@ import { RestTestLibrary } from "../src/testing/index.js"; export async function createRestTestHost(): Promise { return createTestHost({ libraries: [HttpTestLibrary, RestTestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createRestTestRunner(): Promise { diff --git a/packages/samples/specs/versioning/main.tsp b/packages/samples/specs/versioning/main.tsp index ffff9cc5d05..7ba711d0418 100644 --- a/packages/samples/specs/versioning/main.tsp +++ b/packages/samples/specs/versioning/main.tsp @@ -3,7 +3,6 @@ import "@typespec/rest"; import "./library.tsp"; using TypeSpec.Versioning; -using TypeSpec.Rest; @service({ title: "Pet Store Service", diff --git a/packages/samples/specs/visibility/visibility.tsp b/packages/samples/specs/visibility/visibility.tsp index 03f519c3b84..6f0afcb42ef 100644 --- a/packages/samples/specs/visibility/visibility.tsp +++ b/packages/samples/specs/visibility/visibility.tsp @@ -2,7 +2,6 @@ import "@typespec/http"; import "@typespec/rest"; using TypeSpec.Http; -using TypeSpec.Rest; @service({ title: "Visibility sample", diff --git a/packages/xml/test/test-host.ts b/packages/xml/test/test-host.ts index ada33dbc4c9..f0285cd5d73 100644 --- a/packages/xml/test/test-host.ts +++ b/packages/xml/test/test-host.ts @@ -4,6 +4,7 @@ import { XmlTestLibrary } from "../src/testing/index.js"; export async function createXmlTestHost() { return createTestHost({ libraries: [XmlTestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createXmlTestRunner() { From 52298a95b29b7ce72a8791a8aa31b054de5a6bf8 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 12:10:25 +0800 Subject: [PATCH 07/22] fix test and add one more test case --- .../test/checker/unused-using.test.ts | 33 +++++++++++++++++++ packages/openapi/test/test-host.ts | 1 + .../playground-website/samples/unions.tsp | 1 - .../specs/grpc-kiosk-example/types.tsp | 2 -- .../specs/polymorphism/polymorphism.tsp | 1 - .../samples/specs/use-versioned-lib/main.tsp | 1 - packages/streams/test/test-host.ts | 1 + 7 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/compiler/test/checker/unused-using.test.ts b/packages/compiler/test/checker/unused-using.test.ts index 15cf3541905..d64163427d2 100644 --- a/packages/compiler/test/checker/unused-using.test.ts +++ b/packages/compiler/test/checker/unused-using.test.ts @@ -849,4 +849,37 @@ describe("compiler: unused using statements", () => { const diagnostics = await testHost.diagnose("./", { nostdlib: true }); expectDiagnosticEmpty(diagnostics); }); + + it("unused using when type referenced directly", async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./other.tsp"; + namespace Main { + using Other; + + model MainModel { + a: Other.OtherModel; + } + } + `, + ); + testHost.addTypeSpecFile( + "other.tsp", + ` + namespace Other { + model OtherModel { + } + } + `, + ); + const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: "Unused using: using Other", + severity: "hint", + }, + ]); + }); }); diff --git a/packages/openapi/test/test-host.ts b/packages/openapi/test/test-host.ts index c32f07e16bb..ccdff53f622 100644 --- a/packages/openapi/test/test-host.ts +++ b/packages/openapi/test/test-host.ts @@ -6,6 +6,7 @@ import { OpenAPITestLibrary } from "../src/testing/index.js"; export async function createOpenAPITestHost() { return createTestHost({ libraries: [HttpTestLibrary, RestTestLibrary, OpenAPITestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createOpenAPITestRunner() { diff --git a/packages/playground-website/samples/unions.tsp b/packages/playground-website/samples/unions.tsp index 6a6746d1afe..5e07c65fe43 100644 --- a/packages/playground-website/samples/unions.tsp +++ b/packages/playground-website/samples/unions.tsp @@ -6,7 +6,6 @@ import "@typespec/openapi3"; title: "Widget Service", }) namespace DemoService; -using TypeSpec.Rest; using TypeSpec.Http; using TypeSpec.OpenAPI; diff --git a/packages/samples/specs/grpc-kiosk-example/types.tsp b/packages/samples/specs/grpc-kiosk-example/types.tsp index 2c8af4f05a0..6ac52625972 100644 --- a/packages/samples/specs/grpc-kiosk-example/types.tsp +++ b/packages/samples/specs/grpc-kiosk-example/types.tsp @@ -1,7 +1,5 @@ import "@typespec/openapi"; -using TypeSpec.Http; - // // types.tsp // diff --git a/packages/samples/specs/polymorphism/polymorphism.tsp b/packages/samples/specs/polymorphism/polymorphism.tsp index 494bf24852e..f1d8372d31c 100644 --- a/packages/samples/specs/polymorphism/polymorphism.tsp +++ b/packages/samples/specs/polymorphism/polymorphism.tsp @@ -1,7 +1,6 @@ import "@typespec/rest"; using TypeSpec.Http; -using TypeSpec.Rest; @service({ title: "Polymorphism sample", diff --git a/packages/samples/specs/use-versioned-lib/main.tsp b/packages/samples/specs/use-versioned-lib/main.tsp index d52b8b38e41..2f7aea72dbb 100644 --- a/packages/samples/specs/use-versioned-lib/main.tsp +++ b/packages/samples/specs/use-versioned-lib/main.tsp @@ -10,6 +10,5 @@ using TypeSpec.Versioning; }) @useDependency(Library.Versions.`1.0`) namespace VersionedApi; -using TypeSpec.Http; op read(): Library.PetToy; diff --git a/packages/streams/test/test-host.ts b/packages/streams/test/test-host.ts index 1bb51f7ac3a..31907bec5ef 100644 --- a/packages/streams/test/test-host.ts +++ b/packages/streams/test/test-host.ts @@ -4,6 +4,7 @@ import { StreamsTestLibrary } from "../src/testing/index.js"; export async function createStreamsTestHost() { return createTestHost({ libraries: [StreamsTestLibrary], + diagnosticFilter: (diag) => diag.severity !== "hint", }); } From 46ccfca79b7885e2c3cb4682a8329436637c650e Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 13:34:48 +0800 Subject: [PATCH 08/22] fix test --- packages/http/test/test-host.ts | 2 +- packages/rest/test/test-host.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/http/test/test-host.ts b/packages/http/test/test-host.ts index 71c8be3d526..e8f4d7d0139 100644 --- a/packages/http/test/test-host.ts +++ b/packages/http/test/test-host.ts @@ -103,7 +103,7 @@ export async function getOperationsWithServiceNamespace( }, ); const [services] = getAllHttpServices(runner.program, routeOptions); - return [services[0].operations, runner.program.diagnostics]; + return [services[0].operations, runner.program.diagnostics.filter((d) => d.severity !== "hint")]; } export async function getOperations(code: string): Promise { diff --git a/packages/rest/test/test-host.ts b/packages/rest/test/test-host.ts index c9e19239cf7..13151429473 100644 --- a/packages/rest/test/test-host.ts +++ b/packages/rest/test/test-host.ts @@ -96,7 +96,7 @@ export async function getOperationsWithServiceNamespace( }, ); const [services] = getAllHttpServices(runner.program, routeOptions); - return [services[0].operations, runner.program.diagnostics]; + return [services[0].operations, runner.program.diagnostics.filter((d) => d.severity !== "hint")]; } export async function getOperations(code: string): Promise { From 50e995b515ab5dd74eee24610ed619e659c24d04 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 14:14:09 +0800 Subject: [PATCH 09/22] fix e2e test --- packages/http-specs/specs/server/path/multiple/main.tsp | 1 - .../specs/special-headers/conditional-request/main.tsp | 1 - packages/http-specs/specs/special-headers/repeatability/main.tsp | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/http-specs/specs/server/path/multiple/main.tsp b/packages/http-specs/specs/server/path/multiple/main.tsp index 868684aeb39..3535d7c585b 100644 --- a/packages/http-specs/specs/server/path/multiple/main.tsp +++ b/packages/http-specs/specs/server/path/multiple/main.tsp @@ -5,7 +5,6 @@ import "@typespec/versioning"; using Http; using Spector; using TypeSpec.Versioning; -using TypeSpec.Rest; @versioned(Versions) @service({ diff --git a/packages/http-specs/specs/special-headers/conditional-request/main.tsp b/packages/http-specs/specs/special-headers/conditional-request/main.tsp index 3f6b6d8979d..820f2427f55 100644 --- a/packages/http-specs/specs/special-headers/conditional-request/main.tsp +++ b/packages/http-specs/specs/special-headers/conditional-request/main.tsp @@ -4,7 +4,6 @@ import "@typespec/spector"; using Http; using Spector; -using TypeSpec.Versioning; @doc("Illustrates conditional request headers") @scenarioService("/special-headers/conditional-request") diff --git a/packages/http-specs/specs/special-headers/repeatability/main.tsp b/packages/http-specs/specs/special-headers/repeatability/main.tsp index e9bb0067efa..3c7a8511375 100644 --- a/packages/http-specs/specs/special-headers/repeatability/main.tsp +++ b/packages/http-specs/specs/special-headers/repeatability/main.tsp @@ -4,7 +4,6 @@ import "@typespec/spector"; using Http; using Spector; -using TypeSpec.Versioning; @doc("Illustrates OASIS repeatability headers") @scenarioService("/special-headers/repeatability") From 42a3db2a581f9bf0871d7f9eb9a538aba8343c83 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 27 Dec 2024 14:37:40 +0800 Subject: [PATCH 10/22] add changelog --- .../changes/unused-using-2024-11-27-14-37-20.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .chronus/changes/unused-using-2024-11-27-14-37-20.md diff --git a/.chronus/changes/unused-using-2024-11-27-14-37-20.md b/.chronus/changes/unused-using-2024-11-27-14-37-20.md new file mode 100644 index 00000000000..029d3ae02b3 --- /dev/null +++ b/.chronus/changes/unused-using-2024-11-27-14-37-20.md @@ -0,0 +1,14 @@ +--- +changeKind: internal +packages: + - "@typespec/http-specs" + - "@typespec/http" + - "@typespec/openapi" + - "@typespec/openapi3" + - "@typespec/rest" + - "@typespec/streams" + - "@typespec/xml" + - "@typespec/http-server-csharp" +--- + +Fix test for diagnostics of unused using statement \ No newline at end of file From 661740389d41d1f7d5b3e75586f732106b628e0a Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Wed, 15 Jan 2025 21:51:40 +0800 Subject: [PATCH 11/22] refine the message for unused using --- packages/compiler/src/core/messages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index a999b25528d..c35a64b86cd 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -579,7 +579,7 @@ const diagnostics = { "unused-using": { severity: "hint", messages: { - default: paramMessage`Unused using: ${"code"}`, + default: paramMessage`'${"code"}' is declared but never used.`, }, }, From 7d2d58de243fd5e42ea58a6812605431122218ad Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 17 Jan 2025 12:49:26 +0800 Subject: [PATCH 12/22] update test --- .../compiler/test/checker/decorators.test.ts | 4 +- .../test/checker/unused-using.test.ts | 58 +++++++++---------- packages/compiler/test/checker/using.test.ts | 18 +++--- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts index 72e6ff1128a..07419746416 100644 --- a/packages/compiler/test/checker/decorators.test.ts +++ b/packages/compiler/test/checker/decorators.test.ts @@ -46,7 +46,7 @@ describe("compiler: checker: decorators", () => { const unnecessaryDiags: DiagnosticMatch[] = [ { code: "unused-using", - message: `Unused using: using TypeSpec.Reflection`, + message: `'using TypeSpec.Reflection' is declared but never used.`, severity: "hint", }, ]; @@ -182,7 +182,7 @@ describe("compiler: checker: decorators", () => { const unnecessaryDiags: DiagnosticMatch[] = [ { code: "unused-using", - message: `Unused using: using TypeSpec.Reflection`, + message: `'using TypeSpec.Reflection' is declared but never used.`, severity: "hint", }, ]; diff --git a/packages/compiler/test/checker/unused-using.test.ts b/packages/compiler/test/checker/unused-using.test.ts index d64163427d2..47db89862f9 100644 --- a/packages/compiler/test/checker/unused-using.test.ts +++ b/packages/compiler/test/checker/unused-using.test.ts @@ -73,7 +73,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, ]); @@ -109,12 +109,12 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, ]); @@ -151,12 +151,12 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using M", + message: "'using M' is declared but never used.", severity: "hint", }, ]); @@ -193,7 +193,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using M", + message: "'using M' is declared but never used.", severity: "hint", }, ]); @@ -226,7 +226,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using M", + message: "'using M' is declared but never used.", severity: "hint", }, ]); @@ -251,7 +251,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, ]); @@ -288,12 +288,12 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N.M", + message: "'using N.M' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using N.M", + message: "'using N.M' is declared but never used.", severity: "hint", }, ]); @@ -335,12 +335,12 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using TypeSpec.Xyz", + message: "'using TypeSpec.Xyz' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Xyz", + message: "'using Xyz' is declared but never used.", severity: "hint", }, ]); @@ -371,12 +371,12 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N.A", + message: "'using N.A' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using M.A", + message: "'using M.A' is declared but never used.", severity: "hint", }, ]); @@ -443,7 +443,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N.M", + message: "'using N.M' is declared but never used.", severity: "hint", }, ]); @@ -543,7 +543,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using M", + message: "'using M' is declared but never used.", severity: "hint", }, ]); @@ -615,47 +615,47 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using Ns1", + message: "'using Ns1' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2", + message: "'using Ns1.Ns2' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2.Ns3", + message: "'using Ns1.Ns2.Ns3' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2", + message: "'using Ns1.Ns2' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2.Ns3", + message: "'using Ns1.Ns2.Ns3' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1", + message: "'using Ns1' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2.Ns3", + message: "'using Ns1.Ns2.Ns3' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1", + message: "'using Ns1' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using Ns1.Ns2", + message: "'using Ns1.Ns2' is declared but never used.", severity: "hint", }, ]); @@ -730,7 +730,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using TypeSpec", + message: "'using TypeSpec' is declared but never used.", severity: "hint", }, ]); @@ -764,7 +764,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using Other", + message: "'using Other' is declared but never used.", severity: "hint", }, ]); @@ -807,7 +807,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using LibNs", + message: "'using LibNs' is declared but never used.", severity: "hint", }, ]); @@ -877,7 +877,7 @@ describe("compiler: unused using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using Other", + message: "'using Other' is declared but never used.", severity: "hint", }, ]); diff --git a/packages/compiler/test/checker/using.test.ts b/packages/compiler/test/checker/using.test.ts index 13aac35a4e4..7e7d016003c 100644 --- a/packages/compiler/test/checker/using.test.ts +++ b/packages/compiler/test/checker/using.test.ts @@ -127,7 +127,7 @@ describe("compiler: using statements", () => { expectDiagnostics(diags, [ { code: "unused-using", - message: "Unused using: using A", + message: "'using A' is declared but never used.", severity: "hint", }, ]); @@ -197,12 +197,12 @@ describe("compiler: using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N.A", + message: "'using N.A' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using M.A", + message: "'using M.A' is declared but never used.", severity: "hint", }, ]); @@ -230,17 +230,17 @@ describe("compiler: using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using A", + message: "'using A' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using A", + message: "'using A' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using A", + message: "'using A' is declared but never used.", severity: "hint", }, ]); @@ -311,12 +311,12 @@ describe("compiler: using statements", () => { expectDiagnostics(diagnostics, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, { code: "unused-using", - message: "Unused using: using M", + message: "'using M' is declared but never used.", severity: "hint", }, ]); @@ -560,7 +560,7 @@ describe("compiler: using statements", () => { expectDiagnostics(diags, [ { code: "unused-using", - message: "Unused using: using N", + message: "'using N' is declared but never used.", severity: "hint", }, ]); From a68e286d2fa6c777f470e647954667c1e3c38725 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Fri, 17 Jan 2025 16:21:56 +0800 Subject: [PATCH 13/22] fix test --- packages/http/test/streams/get-stream-metadata.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/http/test/streams/get-stream-metadata.test.ts b/packages/http/test/streams/get-stream-metadata.test.ts index 82c62dc8698..56c09eea943 100644 --- a/packages/http/test/streams/get-stream-metadata.test.ts +++ b/packages/http/test/streams/get-stream-metadata.test.ts @@ -20,6 +20,7 @@ let getHttpServiceWithProgram: ( beforeEach(async () => { const host = await createTestHost({ libraries: [StreamsTestLibrary, HttpTestLibrary], + diagnosticFilter: (d) => d.severity !== "hint", }); runner = createTestWrapper(host, { autoImports: [`@typespec/http/streams`, "@typespec/streams"], From 406033832a639273a6a9de369c34000aeff0f587 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 20:47:08 +0800 Subject: [PATCH 14/22] using linter to report unused-using --- packages/compiler/src/core/diagnostics.ts | 4 +- packages/compiler/src/core/linter.ts | 68 +++++++++++++++++-- packages/compiler/src/core/messages.ts | 6 -- packages/compiler/src/core/program.ts | 32 +-------- packages/compiler/src/core/types.ts | 4 +- .../compiler/src/server/compile-service.ts | 21 ++++-- packages/compiler/src/server/diagnostics.ts | 4 +- packages/compiler/src/server/serverlib.ts | 16 ++++- packages/compiler/src/server/types.ts | 9 ++- 9 files changed, 107 insertions(+), 57 deletions(-) diff --git a/packages/compiler/src/core/diagnostics.ts b/packages/compiler/src/core/diagnostics.ts index eb4033ae229..f234a975127 100644 --- a/packages/compiler/src/core/diagnostics.ts +++ b/packages/compiler/src/core/diagnostics.ts @@ -33,7 +33,7 @@ export type DiagnosticHandler = (diagnostic: Diagnostic) => void; export function logDiagnostics(diagnostics: readonly Diagnostic[], logger: LogSink) { for (const diagnostic of diagnostics) { logger.log({ - level: diagnostic.severity === "hint" ? "trace" : diagnostic.severity, + level: diagnostic.severity, message: diagnostic.message, code: diagnostic.code, url: diagnostic.url, @@ -52,7 +52,7 @@ export function formatDiagnostic(diagnostic: Diagnostic, options: FormatDiagnost return formatLog( { code: diagnostic.code, - level: diagnostic.severity === "hint" ? "trace" : diagnostic.severity, + level: diagnostic.severity, message: diagnostic.message, url: diagnostic.url, sourceLocation: getSourceLocation(diagnostic.target, { locateId: true }), diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index 4cc20a8cba6..b74f59e47ba 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -1,23 +1,30 @@ +import { removeUnusedCodeCodeFix } from "./compiler-code-fixes/remove-unused-code.codefix.js"; import { DiagnosticCollector, compilerAssert, createDiagnosticCollector } from "./diagnostics.js"; import { getLocationContext } from "./helpers/location-context.js"; +import { createLinterRule, defineLinter, paramMessage } from "./library.js"; import { createDiagnostic } from "./messages.js"; +import { NameResolver } from "./name-resolver.js"; import type { Program } from "./program.js"; import { EventEmitter, mapEventEmitterToNodeListener, navigateProgram } from "./semantic-walker.js"; import { Diagnostic, DiagnosticMessages, - LibraryInstance, + IdentifierNode, LinterDefinition, LinterResolvedDefinition, LinterRule, LinterRuleContext, LinterRuleDiagnosticReport, LinterRuleSet, + MemberExpressionNode, NoTarget, RuleRef, SemanticNodeListener, + SyntaxKind, } from "./types.js"; +type LinterLibraryInstance = { linter: LinterResolvedDefinition }; + export interface Linter { extendRuleSet(ruleSet: LinterRuleSet): Promise; lint(): readonly Diagnostic[]; @@ -53,13 +60,18 @@ export function resolveLinterDefinition( export function createLinter( program: Program, - loadLibrary: (name: string) => Promise, + nameResolver: NameResolver, + loadLibrary: (name: string) => Promise, ): Linter { const tracer = program.tracer.sub("linter"); const ruleMap = new Map>(); const enabledRules = new Map>(); - const linterLibraries = new Map(); + const linterLibraries = new Map(); + const builtInLinter: LinterResolvedDefinition = resolveLinterDefinition( + builtInLinterLibraryName, + createDefaultLinter(nameResolver), + ); return { extendRuleSet, @@ -158,7 +170,7 @@ export function createLinter( return diagnostics.diagnostics; } - async function resolveLibrary(name: string): Promise { + async function resolveLibrary(name: string): Promise { const loadedLibrary = linterLibraries.get(name); if (loadedLibrary === undefined) { return registerLinterLibrary(name); @@ -166,10 +178,11 @@ export function createLinter( return loadedLibrary; } - async function registerLinterLibrary(name: string): Promise { + async function registerLinterLibrary(name: string): Promise { tracer.trace("register-library", name); - const library = await loadLibrary(name); + const library = + name === builtInLinterLibraryName ? { linter: builtInLinter } : await loadLibrary(name); const linter = library?.linter; if (linter?.rules) { for (const rule of linter.rules) { @@ -253,3 +266,46 @@ export function createLinterRuleContext { + const getUsingName = (node: MemberExpressionNode | IdentifierNode): string => { + if (node.kind === SyntaxKind.MemberExpression) { + return `${getUsingName(node.base)}${node.selector}${node.id.sv}`; + } else { + // identifier node + return node.sv; + } + }; + nameResolver.getUnusedUsings().forEach((target) => { + context.reportDiagnostic({ + messageId: "default", + format: { code: getUsingName(target.name) }, + target, + codefixes: [removeUnusedCodeCodeFix(target)], + }); + }); + }, + }; + }, + }); + } +} diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index c35a64b86cd..a0b74d2d583 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -576,12 +576,6 @@ const diagnostics = { default: "The #deprecated directive cannot be used more than once on the same declaration.", }, }, - "unused-using": { - severity: "hint", - messages: { - default: paramMessage`'${"code"}' is declared but never used.`, - }, - }, /** * Configuration diff --git a/packages/compiler/src/core/program.ts b/packages/compiler/src/core/program.ts index 980a3b2ad23..2f14e73061f 100644 --- a/packages/compiler/src/core/program.ts +++ b/packages/compiler/src/core/program.ts @@ -15,7 +15,6 @@ import { PackageJson } from "../types/package-json.js"; import { deepEquals, findProjectRoot, isDefined, mapEquals, mutate } from "../utils/misc.js"; import { createBinder } from "./binder.js"; import { Checker, createChecker } from "./checker.js"; -import { removeUnusedCodeCodeFix } from "./compiler-code-fixes/remove-unused-code.codefix.js"; import { createSuppressCodeFix } from "./compiler-code-fixes/suppress.codefix.js"; import { compilerAssert } from "./diagnostics.js"; import { resolveTypeSpecEntrypoint } from "./entrypoint-resolution.js"; @@ -46,13 +45,11 @@ import { EmitContext, EmitterFunc, Entity, - IdentifierNode, JsSourceFileNode, LibraryInstance, LibraryMetadata, LiteralType, LocationContext, - MemberExpressionNode, ModuleLibraryMetadata, Namespace, NoTarget, @@ -230,12 +227,12 @@ export async function compile( oldProgram = undefined; setCurrentProgram(program); - const linter = createLinter(program, (name) => loadLibrary(basedir, name)); + const resolver = createResolver(program); + const linter = createLinter(program, resolver, (name) => loadLibrary(basedir, name)); if (options.linterRuleSet) { program.reportDiagnostics(await linter.extendRuleSet(options.linterRuleSet)); } - const resolver = createResolver(program); resolver.resolveProgram(); program.checker = createChecker(program, resolver); program.checker.checkProgram(); @@ -253,8 +250,6 @@ export async function compile( return program; } - validateUnusedCode(); - // Linter stage program.reportDiagnostics(linter.lint()); @@ -265,29 +260,6 @@ export async function compile( return program; - function validateUnusedCode() { - const getUsingName = (node: MemberExpressionNode | IdentifierNode): string => { - if (node.kind === SyntaxKind.MemberExpression) { - return `${getUsingName(node.base)}${node.selector}${node.id.sv}`; - } else { - // identifier node - return node.sv; - } - }; - resolver.getUnusedUsings().forEach((target) => { - reportDiagnostic( - createDiagnostic({ - code: "unused-using", - target: target, - format: { - code: `using ${getUsingName(target.name)}`, - }, - codefixes: [removeUnusedCodeCodeFix(target)], - }), - ); - }); - } - /** * Validate the libraries loaded during the compilation process are compatible. */ diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index 93803cbdb7b..620d5c52f43 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -2300,7 +2300,7 @@ export interface TemplateInstanceTarget { export type DiagnosticTarget = TypeSpecDiagnosticTarget | SourceLocation; -export type DiagnosticSeverity = "error" | "warning" | "hint"; +export type DiagnosticSeverity = "error" | "warning"; export interface Diagnostic { code: string; @@ -2531,7 +2531,7 @@ export interface DiagnosticDefinition { * - `error` - Non-suppressable, should be used to represent failure to move forward. * - `hint` - Something to hint to a better way of doing it, like proposing a refactoring. */ - readonly severity: "warning" | "error" | "hint"; + readonly severity: "warning" | "error"; /** Messages that can be reported with the diagnostic. */ readonly messages: M; /** Short description of the diagnostic */ diff --git a/packages/compiler/src/server/compile-service.ts b/packages/compiler/src/server/compile-service.ts index f2fa42c6ea2..6017352a83a 100644 --- a/packages/compiler/src/server/compile-service.ts +++ b/packages/compiler/src/server/compile-service.ts @@ -19,6 +19,7 @@ import { joinPaths, parse, } from "../core/index.js"; +import { builtInLinterLibraryName, builtInLinterRule_UnusedUsing } from "../core/linter.js"; import { compile as compileProgram } from "../core/program.js"; import { doIO, loadFile, resolveTspMain } from "../utils/misc.js"; import { serverOptions } from "./constants.js"; @@ -107,6 +108,18 @@ export function createCompileService({ ...optionsFromConfig, ...serverOptions, }; + // add linter rule for unused using if user didn't configure it explicitly + if ( + options.linterRuleSet?.enable?.[ + `${builtInLinterLibraryName}/${builtInLinterRule_UnusedUsing}` + ] === undefined + ) { + options.linterRuleSet ??= {}; + options.linterRuleSet.enable ??= {}; + options.linterRuleSet.enable[`${builtInLinterLibraryName}/${builtInLinterRule_UnusedUsing}`] = + true; + } + log({ level: "debug", message: `compiler options resolved`, detail: options }); if (!fileService.upToDate(document)) { @@ -142,7 +155,7 @@ export function createCompileService({ const script = program.sourceFiles.get(resolvedPath); compilerAssert(script, "Failed to get script."); - const result: CompileResult = { program, document: doc, script }; + const result: CompileResult = { program, document: doc, script, optionsFromConfig }; notify("compileEnd", result); return result; } catch (err: any) { @@ -180,13 +193,13 @@ export function createCompileService({ } const cached = await fileSystemCache.get(configPath); + const deepCopy = (obj: any) => JSON.parse(JSON.stringify(obj)); if (cached?.data) { - return cached.data; + return deepCopy(cached.data); } const config = await loadTypeSpecConfigFile(compilerHost, configPath); - await fileSystemCache.setData(configPath, config); - return config; + return deepCopy(config); } async function getScript( diff --git a/packages/compiler/src/server/diagnostics.ts b/packages/compiler/src/server/diagnostics.ts index 5c5f6876767..f9dbe4718a2 100644 --- a/packages/compiler/src/server/diagnostics.ts +++ b/packages/compiler/src/server/diagnostics.ts @@ -141,13 +141,11 @@ function getVSLocationWithTypeInfo( }; } -function convertSeverity(severity: "warning" | "error" | "hint"): DiagnosticSeverity { +function convertSeverity(severity: "warning" | "error"): DiagnosticSeverity { switch (severity) { case "warning": return DiagnosticSeverity.Warning; case "error": return DiagnosticSeverity.Error; - case "hint": - return DiagnosticSeverity.Hint; } } diff --git a/packages/compiler/src/server/serverlib.ts b/packages/compiler/src/server/serverlib.ts index 8370c36d36d..3cbc6107997 100644 --- a/packages/compiler/src/server/serverlib.ts +++ b/packages/compiler/src/server/serverlib.ts @@ -6,6 +6,7 @@ import { CompletionList, CompletionParams, DefinitionParams, + DiagnosticSeverity, DiagnosticTag, DidChangeWatchedFilesParams, DocumentFormattingParams, @@ -57,6 +58,7 @@ import { ResolveModuleHost, typespecVersion, } from "../core/index.js"; +import { builtInLinterLibraryName, builtInLinterRule_UnusedUsing } from "../core/linter.js"; import { formatLog } from "../core/logger/index.js"; import { getPositionBeforeTrivia } from "../core/parser-utils.js"; import { getNodeAtPosition, getNodeAtPositionDetail, visitChildren } from "../core/parser.js"; @@ -324,7 +326,7 @@ export function createServer(host: ServerHost): Server { if (!validationResult.valid) { for (const diag of validationResult.diagnostics) { log({ - level: diag.severity === "hint" ? "info" : diag.severity, + level: diag.severity, message: diag.message, detail: { code: diag.code, @@ -467,7 +469,7 @@ export function createServer(host: ServerHost): Server { compileService.notifyChange(change.document); } - async function reportDiagnostics({ program, document }: CompileResult) { + async function reportDiagnostics({ program, document, optionsFromConfig }: CompileResult) { if (isTspConfigFile(document)) return undefined; currentDiagnosticIndex.clear(); @@ -497,11 +499,19 @@ export function createServer(host: ServerHost): Server { } if (each.code === "deprecated") { diagnostic.tags = [DiagnosticTag.Deprecated]; - } else if (each.code === "unused-using") { + } else if (each.code === `${builtInLinterLibraryName}/${builtInLinterRule_UnusedUsing}`) { // Unused or unnecessary code. Diagnostics with this tag are rendered faded out, so no extra work needed from IDE side // https://vscode-api.js.org/enums/vscode.DiagnosticTag.html#google_vignette // https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.languageserver.protocol.diagnostictag?view=visualstudiosdk-2022 diagnostic.tags = [DiagnosticTag.Unnecessary]; + if ( + optionsFromConfig.linterRuleSet?.enable?.[ + `${builtInLinterLibraryName}/${builtInLinterRule_UnusedUsing}` + ] === undefined + ) { + // if the unused using is not configured by user explicitly, report it as hint by default + diagnostic.severity = DiagnosticSeverity.Hint; + } } diagnostic.data = { id: diagnosticIdCounter++ }; const diagnostics = diagnosticMap.get(diagDocument); diff --git a/packages/compiler/src/server/types.ts b/packages/compiler/src/server/types.ts index 7601637da07..ab4ed6894c3 100644 --- a/packages/compiler/src/server/types.ts +++ b/packages/compiler/src/server/types.ts @@ -37,7 +37,13 @@ import { WorkspaceFoldersChangeEvent, } from "vscode-languageserver"; import { TextDocument, TextEdit } from "vscode-languageserver-textdocument"; -import type { CompilerHost, Program, SourceFile, TypeSpecScriptNode } from "../core/index.js"; +import type { + CompilerHost, + CompilerOptions, + Program, + SourceFile, + TypeSpecScriptNode, +} from "../core/index.js"; import { LoadedCoreTemplates } from "../init/core-templates.js"; import { EmitterTemplate, InitTemplate, InitTemplateLibrarySpec } from "../init/init-template.js"; import { ScaffoldingConfig } from "../init/scaffold.js"; @@ -64,6 +70,7 @@ export interface CompileResult { readonly program: Program; readonly document: TextDocument; readonly script: TypeSpecScriptNode; + readonly optionsFromConfig: CompilerOptions; } export interface Server { From 4df823c3711872125b773b3424bcc446f0ede3af Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 20:48:39 +0800 Subject: [PATCH 15/22] update test --- packages/compiler/test/core/linter.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts index a7d467752b1..8dca9150432 100644 --- a/packages/compiler/test/core/linter.test.ts +++ b/packages/compiler/test/core/linter.test.ts @@ -2,6 +2,7 @@ import { describe, it } from "vitest"; import { createLinterRule, createTypeSpecLibrary } from "../../src/core/library.js"; import { Linter, createLinter, resolveLinterDefinition } from "../../src/core/linter.js"; +import { createResolver } from "../../src/core/name-resolver.js"; import type { LibraryInstance, LinterDefinition } from "../../src/index.js"; import { createTestHost, @@ -56,7 +57,8 @@ describe("compiler: linter", () => { await host.compile("main.tsp"); - const linter = createLinter(host.program, (libName) => + const resolver = createResolver(host.program); + const linter = createLinter(host.program, resolver, (libName) => Promise.resolve(libName === "@typespec/test-linter" ? library : undefined), ); return linter; From 3eaf6ab39347986abea2db4519396667ac0bb014 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 21:36:30 +0800 Subject: [PATCH 16/22] update tests --- .../compiler/test/checker/decorators.test.ts | 130 +++++-------- packages/compiler/test/checker/using.test.ts | 180 ++++++------------ packages/http-server-csharp/test/test-host.ts | 1 - .../specs/server/path/multiple/main.tsp | 1 + .../conditional-request/main.tsp | 1 + .../special-headers/repeatability/main.tsp | 1 + .../test/streams/get-stream-metadata.test.ts | 1 - packages/http/test/test-host.ts | 3 +- packages/openapi/test/test-host.ts | 1 - packages/openapi3/test/test-host.ts | 1 - .../playground-website/samples/unions.tsp | 1 + packages/rest/test/test-host.ts | 3 +- .../specs/grpc-kiosk-example/types.tsp | 2 + .../specs/polymorphism/polymorphism.tsp | 1 + .../samples/specs/use-versioned-lib/main.tsp | 1 + packages/samples/specs/versioning/main.tsp | 1 + .../samples/specs/visibility/visibility.tsp | 1 + packages/streams/test/test-host.ts | 1 - packages/xml/test/test-host.ts | 1 - 19 files changed, 110 insertions(+), 222 deletions(-) diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts index 07419746416..41e05c83f18 100644 --- a/packages/compiler/test/checker/decorators.test.ts +++ b/packages/compiler/test/checker/decorators.test.ts @@ -11,7 +11,6 @@ import { numericRanges } from "../../src/core/numeric-ranges.js"; import { Numeric } from "../../src/core/numeric.js"; import { BasicTestRunner, - DiagnosticMatch, TestHost, createTestHost, createTestWrapper, @@ -43,18 +42,6 @@ describe("compiler: checker: decorators", () => { }); }); - const unnecessaryDiags: DiagnosticMatch[] = [ - { - code: "unused-using", - message: `'using TypeSpec.Reflection' is declared but never used.`, - severity: "hint", - }, - ]; - const compileAndCheckDiags = async (code: string) => { - const [_, diags] = await runner.compileAndDiagnose(code); - expectDiagnostics(diags, [...unnecessaryDiags]); - }; - describe("bind implementation to declaration", () => { let $otherDec: DecoratorFunction; function expectDecorator(ns: Namespace) { @@ -70,7 +57,7 @@ describe("compiler: checker: decorators", () => { }); it("defined at root", async () => { - await compileAndCheckDiags(` + await runner.compile(` extern dec otherDec(target: unknown); `); @@ -80,7 +67,7 @@ describe("compiler: checker: decorators", () => { it("in a namespace", async () => { setTypeSpecNamespace("Foo.Bar", $otherDec); - await compileAndCheckDiags(` + await runner.compile(` namespace Foo.Bar { extern dec otherDec(target: unknown); } @@ -99,7 +86,7 @@ describe("compiler: checker: decorators", () => { it("defined at root", async () => { testJs.$decorators = { "": { otherDec: $otherDec } }; - await compileAndCheckDiags(` + await runner.compile(` extern dec otherDec(target: unknown); `); @@ -109,7 +96,7 @@ describe("compiler: checker: decorators", () => { it("in a namespace", async () => { testJs.$decorators = { "Foo.Bar": { otherDec: $otherDec } }; - await compileAndCheckDiags(` + await runner.compile(` namespace Foo.Bar { extern dec otherDec(target: unknown); } @@ -129,36 +116,30 @@ describe("compiler: checker: decorators", () => { const diagnostics = await runner.diagnose(` dec testDec(target: unknown); `); - expectDiagnostics(diagnostics, [ - { - code: "decorator-extern", - message: "A decorator declaration must be prefixed with the 'extern' modifier.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "decorator-extern", + message: "A decorator declaration must be prefixed with the 'extern' modifier.", + }); }); it("errors if rest parameter type is not an array expression", async () => { const diagnostics = await runner.diagnose(` extern dec testDec(target: unknown, ...rest: string); `); - expectDiagnostics(diagnostics, [ - { - code: "rest-parameter-array", - message: "A rest parameter must be of an array type.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "rest-parameter-array", + message: "A rest parameter must be of an array type.", + }); }); it("errors if extern decorator is missing implementation", async () => { const diagnostics = await runner.diagnose(` extern dec notImplemented(target: unknown); `); - expectDiagnostics(diagnostics, [ - { - code: "missing-implementation", - message: "Extern declaration must have an implementation in JS file.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "missing-implementation", + message: "Extern declaration must have an implementation in JS file.", + }); }); }); @@ -179,19 +160,6 @@ describe("compiler: checker: decorators", () => { }); }); - const unnecessaryDiags: DiagnosticMatch[] = [ - { - code: "unused-using", - message: `'using TypeSpec.Reflection' is declared but never used.`, - severity: "hint", - }, - ]; - const compileAndCheckDiags = async (code: string) => { - const [r, diags] = await runner.compileAndDiagnose(code); - expectDiagnostics(diags, [...unnecessaryDiags]); - return r; - }; - function expectDecoratorCalledWith(target: unknown, ...args: unknown[]) { ok(calledArgs, "Decorator was not called."); strictEqual(calledArgs.length, 2 + args.length); @@ -207,7 +175,7 @@ describe("compiler: checker: decorators", () => { } it("calls a decorator with no argument", async () => { - const { Foo } = await compileAndCheckDiags(` + const { Foo } = await runner.compile(` extern dec testDec(target: unknown); @testDec @@ -219,7 +187,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with arguments", async () => { - const { Foo } = await compileAndCheckDiags(` + const { Foo } = await runner.compile(` extern dec testDec(target: unknown, arg1: valueof string, arg2: valueof string); @testDec("one", "two") @@ -231,7 +199,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with optional arguments", async () => { - const { Foo } = await compileAndCheckDiags(` + const { Foo } = await runner.compile(` extern dec testDec(target: unknown, arg1: valueof string, arg2?: valueof string); @testDec("one") @@ -243,7 +211,7 @@ describe("compiler: checker: decorators", () => { }); it("calls a decorator with rest arguments", async () => { - const { Foo } = await compileAndCheckDiags(` + const { Foo } = await runner.compile(` extern dec testDec(target: unknown, arg1: valueof string, ...args: valueof string[]); @testDec("one", "two", "three", "four") @@ -262,12 +230,10 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-argument-count", - message: "Expected 2 arguments, but got 1.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-argument-count", + message: "Expected 2 arguments, but got 1.", + }); expectDecoratorNotCalled(); }); @@ -279,12 +245,10 @@ describe("compiler: checker: decorators", () => { @test model Foo {} `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-argument-count", - message: "Expected 1-2 arguments, but got 3.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-argument-count", + message: "Expected 1-2 arguments, but got 3.", + }); expectDecoratorCalledWith(Foo, "one", "two"); }); @@ -296,12 +260,10 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-argument-count", - message: "Expected 0 arguments, but got 1.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-argument-count", + message: "Expected 0 arguments, but got 1.", + }); }); it("errors if not calling with too few arguments with rest", async () => { @@ -312,12 +274,10 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-argument-count", - message: "Expected at least 1 arguments, but got 0.", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-argument-count", + message: "Expected at least 1 arguments, but got 0.", + }); expectDecoratorNotCalled(); }); @@ -344,12 +304,10 @@ describe("compiler: checker: decorators", () => { model Foo {} `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-argument", - message: "Argument of type '123' is not assignable to parameter of type 'string'", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-argument", + message: "Argument of type '123' is not assignable to parameter of type 'string'", + }); expectDecoratorNotCalled(); }); @@ -376,7 +334,7 @@ describe("compiler: checker: decorators", () => { // Regresssion test for https://github.com/microsoft/typespec/issues/3211 it("augmenting a template model property before a decorator declaration resolve the declaration correctly", async () => { - await compileAndCheckDiags(` + await runner.compile(` model Foo { prop: T; } @@ -395,7 +353,7 @@ describe("compiler: checker: decorators", () => { value: string, suppress?: boolean, ): Promise { - await compileAndCheckDiags(` + await runner.compile(` extern dec testDec(target: unknown, arg1: ${type}); ${suppress ? `#suppress "deprecated" "for testing"` : ""} @@ -569,9 +527,7 @@ describe("compiler: checker: decorators", () => { @test model Foo {} `); - expectDiagnosticEmpty( - diagnostics.filter((x) => x.code !== "deprecated" && x.code !== "unused-using"), - ); + expectDiagnosticEmpty(diagnostics.filter((x) => x.code !== "deprecated")); return calledArgs![2]; } diff --git a/packages/compiler/test/checker/using.test.ts b/packages/compiler/test/checker/using.test.ts index 7e7d016003c..736e4b5052e 100644 --- a/packages/compiler/test/checker/using.test.ts +++ b/packages/compiler/test/checker/using.test.ts @@ -1,7 +1,12 @@ import { rejects, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { Model } from "../../src/core/types.js"; -import { TestHost, createTestHost, expectDiagnostics } from "../../src/testing/index.js"; +import { + TestHost, + createTestHost, + expectDiagnosticEmpty, + expectDiagnostics, +} from "../../src/testing/index.js"; describe("compiler: using statements", () => { let testHost: TestHost; @@ -16,7 +21,6 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; - alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -47,7 +51,6 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; - alias foo = Z.Y; `, ); testHost.addTypeSpecFile( @@ -79,7 +82,6 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; - alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -123,14 +125,8 @@ describe("compiler: using statements", () => { `, ); testHost.addTypeSpecFile("b.tsp", `namespace B { model BModel {} }`); - const diags = await testHost.diagnose("./"); - expectDiagnostics(diags, [ - { - code: "unused-using", - message: "'using A' is declared but never used.", - severity: "hint", - }, - ]); + + expectDiagnosticEmpty(await testHost.diagnose("./")); }); it("TypeSpec.Xyz namespace doesn't need TypeSpec prefix in using", async () => { @@ -139,7 +135,6 @@ describe("compiler: using statements", () => { ` import "./a.tsp"; import "./b.tsp"; - alias foo = Y; `, ); testHost.addTypeSpecFile( @@ -194,18 +189,7 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "unused-using", - message: "'using N.A' is declared but never used.", - severity: "hint", - }, - { - code: "unused-using", - message: "'using M.A' is declared but never used.", - severity: "hint", - }, - ]); + expectDiagnosticEmpty(diagnostics); }); describe("duplicate usings", () => { @@ -227,23 +211,7 @@ describe("compiler: using statements", () => { testHost.addTypeSpecFile("a.tsp", `namespace A { model AModel {} }`); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "unused-using", - message: "'using A' is declared but never used.", - severity: "hint", - }, - { - code: "unused-using", - message: "'using A' is declared but never used.", - severity: "hint", - }, - { - code: "unused-using", - message: "'using A' is declared but never used.", - severity: "hint", - }, - ]); + expectDiagnosticEmpty(diagnostics); }); it("throws errors for duplicate imported usings", async () => { @@ -308,18 +276,7 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics(diagnostics, [ - { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", - }, - { - code: "unused-using", - message: "'using M' is declared but never used.", - severity: "hint", - }, - ]); + expectDiagnosticEmpty(diagnostics); }); it("report ambiguous diagnostics when using name present in multiple using", async () => { @@ -353,16 +310,13 @@ describe("compiler: using statements", () => { `, ); const diagnostics = await testHost.diagnose("./", { nostdlib: true }); - expectDiagnostics( - diagnostics.filter((d) => d.code !== "unused-using"), - [ - { - code: "ambiguous-symbol", - message: - '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', - }, - ], - ); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: + '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', + }, + ]); }); it("report ambiguous diagnostics when symbol exists in using namespace and global namespace", async () => { @@ -393,16 +347,13 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./", { nostdlib: true }); - expectDiagnostics( - diagnostics.filter((d) => d.code !== "unused-using"), - [ - { - code: "ambiguous-symbol", - message: - '"M" is an ambiguous name between global.M, B.M. Try using fully qualified name instead: global.M, B.M', - }, - ], - ); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: + '"M" is an ambiguous name between global.M, B.M. Try using fully qualified name instead: global.M, B.M', + }, + ]); }); it("reports ambiguous symbol for decorator", async () => { @@ -429,15 +380,12 @@ describe("compiler: using statements", () => { }); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics( - diagnostics.filter((d) => d.code !== "unused-using"), - [ - { - code: "ambiguous-symbol", - message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, - }, - ], - ); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, + }, + ]); }); it("reports ambiguous symbol for decorator with missing implementation", async () => { @@ -458,16 +406,13 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics( - diagnostics.filter((d) => d.code !== "unused-using"), - [ - { - code: "ambiguous-symbol", - message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, - }, - { code: "missing-implementation" }, - ], - ); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: `"doc" is an ambiguous name between TypeSpec.doc, Test.A.doc. Try using fully qualified name instead: TypeSpec.doc, Test.A.doc`, + }, + { code: "missing-implementation" }, + ]); }); it("ambiguous use doesn't affect other files", async () => { @@ -511,17 +456,14 @@ describe("compiler: using statements", () => { `, ); const diagnostics = await testHost.diagnose("./"); - expectDiagnostics( - diagnostics.filter((d) => d.code !== "unused-using"), - [ - { - code: "ambiguous-symbol", - message: - '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', - file: /ambiguous\.tsp$/, - }, - ], - ); + expectDiagnostics(diagnostics, [ + { + code: "ambiguous-symbol", + message: + '"A" is an ambiguous name between N.A, M.A. Try using fully qualified name instead: N.A, M.A', + file: /ambiguous\.tsp$/, + }, + ]); }); it("resolves 'local' decls over usings", async () => { @@ -551,19 +493,11 @@ describe("compiler: using statements", () => { `, ); - const [result, diags] = await testHost.compileAndDiagnose("./"); - const { B } = result as { + const { B } = (await testHost.compile("./")) as { B: Model; }; strictEqual(B.properties.size, 1); strictEqual(B.properties.get("a")!.type.kind, "Union"); - expectDiagnostics(diags, [ - { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", - }, - ]); }); it("usings are local to a file", async () => { @@ -651,12 +585,10 @@ describe("emit diagnostics", () => { const diagnostics = await diagnose(` using NotDefined; `); - expectDiagnostics(diagnostics, [ - { - code: "invalid-ref", - message: "Unknown identifier NotDefined", - }, - ]); + expectDiagnostics(diagnostics, { + code: "invalid-ref", + message: "Unknown identifier NotDefined", + }); }); describe("when using non-namespace types", () => { @@ -673,12 +605,10 @@ describe("emit diagnostics", () => { using Target; ${code} `); - expectDiagnostics(diagnostics, [ - { - code: "using-invalid-ref", - message: "Using must refer to a namespace", - }, - ]); + expectDiagnostics(diagnostics, { + code: "using-invalid-ref", + message: "Using must refer to a namespace", + }); }); }); }); diff --git a/packages/http-server-csharp/test/test-host.ts b/packages/http-server-csharp/test/test-host.ts index d8bdff3edb4..9fee46b7ca5 100644 --- a/packages/http-server-csharp/test/test-host.ts +++ b/packages/http-server-csharp/test/test-host.ts @@ -13,7 +13,6 @@ export async function createCSharpServiceEmitterTestHost() { VersioningTestLibrary, CSharpServiceEmitterTestLibrary, ], - diagnosticFilter: (diag) => diag.severity !== "hint", }); return result; diff --git a/packages/http-specs/specs/server/path/multiple/main.tsp b/packages/http-specs/specs/server/path/multiple/main.tsp index 3535d7c585b..868684aeb39 100644 --- a/packages/http-specs/specs/server/path/multiple/main.tsp +++ b/packages/http-specs/specs/server/path/multiple/main.tsp @@ -5,6 +5,7 @@ import "@typespec/versioning"; using Http; using Spector; using TypeSpec.Versioning; +using TypeSpec.Rest; @versioned(Versions) @service({ diff --git a/packages/http-specs/specs/special-headers/conditional-request/main.tsp b/packages/http-specs/specs/special-headers/conditional-request/main.tsp index 820f2427f55..3f6b6d8979d 100644 --- a/packages/http-specs/specs/special-headers/conditional-request/main.tsp +++ b/packages/http-specs/specs/special-headers/conditional-request/main.tsp @@ -4,6 +4,7 @@ import "@typespec/spector"; using Http; using Spector; +using TypeSpec.Versioning; @doc("Illustrates conditional request headers") @scenarioService("/special-headers/conditional-request") diff --git a/packages/http-specs/specs/special-headers/repeatability/main.tsp b/packages/http-specs/specs/special-headers/repeatability/main.tsp index 3c7a8511375..e9bb0067efa 100644 --- a/packages/http-specs/specs/special-headers/repeatability/main.tsp +++ b/packages/http-specs/specs/special-headers/repeatability/main.tsp @@ -4,6 +4,7 @@ import "@typespec/spector"; using Http; using Spector; +using TypeSpec.Versioning; @doc("Illustrates OASIS repeatability headers") @scenarioService("/special-headers/repeatability") diff --git a/packages/http/test/streams/get-stream-metadata.test.ts b/packages/http/test/streams/get-stream-metadata.test.ts index 56c09eea943..82c62dc8698 100644 --- a/packages/http/test/streams/get-stream-metadata.test.ts +++ b/packages/http/test/streams/get-stream-metadata.test.ts @@ -20,7 +20,6 @@ let getHttpServiceWithProgram: ( beforeEach(async () => { const host = await createTestHost({ libraries: [StreamsTestLibrary, HttpTestLibrary], - diagnosticFilter: (d) => d.severity !== "hint", }); runner = createTestWrapper(host, { autoImports: [`@typespec/http/streams`, "@typespec/streams"], diff --git a/packages/http/test/test-host.ts b/packages/http/test/test-host.ts index e8f4d7d0139..e3306f1eefa 100644 --- a/packages/http/test/test-host.ts +++ b/packages/http/test/test-host.ts @@ -18,7 +18,6 @@ import { HttpTestLibrary } from "../src/testing/index.js"; export async function createHttpTestHost(): Promise { return createTestHost({ libraries: [HttpTestLibrary], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createHttpTestRunner(): Promise { @@ -103,7 +102,7 @@ export async function getOperationsWithServiceNamespace( }, ); const [services] = getAllHttpServices(runner.program, routeOptions); - return [services[0].operations, runner.program.diagnostics.filter((d) => d.severity !== "hint")]; + return [services[0].operations, runner.program.diagnostics]; } export async function getOperations(code: string): Promise { diff --git a/packages/openapi/test/test-host.ts b/packages/openapi/test/test-host.ts index ccdff53f622..c32f07e16bb 100644 --- a/packages/openapi/test/test-host.ts +++ b/packages/openapi/test/test-host.ts @@ -6,7 +6,6 @@ import { OpenAPITestLibrary } from "../src/testing/index.js"; export async function createOpenAPITestHost() { return createTestHost({ libraries: [HttpTestLibrary, RestTestLibrary, OpenAPITestLibrary], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createOpenAPITestRunner() { diff --git a/packages/openapi3/test/test-host.ts b/packages/openapi3/test/test-host.ts index c51a6ac5d58..84b9655b89c 100644 --- a/packages/openapi3/test/test-host.ts +++ b/packages/openapi3/test/test-host.ts @@ -28,7 +28,6 @@ export async function createOpenAPITestHost() { OpenAPITestLibrary, OpenAPI3TestLibrary, ], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } diff --git a/packages/playground-website/samples/unions.tsp b/packages/playground-website/samples/unions.tsp index 5e07c65fe43..6a6746d1afe 100644 --- a/packages/playground-website/samples/unions.tsp +++ b/packages/playground-website/samples/unions.tsp @@ -6,6 +6,7 @@ import "@typespec/openapi3"; title: "Widget Service", }) namespace DemoService; +using TypeSpec.Rest; using TypeSpec.Http; using TypeSpec.OpenAPI; diff --git a/packages/rest/test/test-host.ts b/packages/rest/test/test-host.ts index 13151429473..564e0485068 100644 --- a/packages/rest/test/test-host.ts +++ b/packages/rest/test/test-host.ts @@ -19,7 +19,6 @@ import { RestTestLibrary } from "../src/testing/index.js"; export async function createRestTestHost(): Promise { return createTestHost({ libraries: [HttpTestLibrary, RestTestLibrary], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createRestTestRunner(): Promise { @@ -96,7 +95,7 @@ export async function getOperationsWithServiceNamespace( }, ); const [services] = getAllHttpServices(runner.program, routeOptions); - return [services[0].operations, runner.program.diagnostics.filter((d) => d.severity !== "hint")]; + return [services[0].operations, runner.program.diagnostics]; } export async function getOperations(code: string): Promise { diff --git a/packages/samples/specs/grpc-kiosk-example/types.tsp b/packages/samples/specs/grpc-kiosk-example/types.tsp index 6ac52625972..2c8af4f05a0 100644 --- a/packages/samples/specs/grpc-kiosk-example/types.tsp +++ b/packages/samples/specs/grpc-kiosk-example/types.tsp @@ -1,5 +1,7 @@ import "@typespec/openapi"; +using TypeSpec.Http; + // // types.tsp // diff --git a/packages/samples/specs/polymorphism/polymorphism.tsp b/packages/samples/specs/polymorphism/polymorphism.tsp index f1d8372d31c..494bf24852e 100644 --- a/packages/samples/specs/polymorphism/polymorphism.tsp +++ b/packages/samples/specs/polymorphism/polymorphism.tsp @@ -1,6 +1,7 @@ import "@typespec/rest"; using TypeSpec.Http; +using TypeSpec.Rest; @service({ title: "Polymorphism sample", diff --git a/packages/samples/specs/use-versioned-lib/main.tsp b/packages/samples/specs/use-versioned-lib/main.tsp index 2f7aea72dbb..d52b8b38e41 100644 --- a/packages/samples/specs/use-versioned-lib/main.tsp +++ b/packages/samples/specs/use-versioned-lib/main.tsp @@ -10,5 +10,6 @@ using TypeSpec.Versioning; }) @useDependency(Library.Versions.`1.0`) namespace VersionedApi; +using TypeSpec.Http; op read(): Library.PetToy; diff --git a/packages/samples/specs/versioning/main.tsp b/packages/samples/specs/versioning/main.tsp index 7ba711d0418..ffff9cc5d05 100644 --- a/packages/samples/specs/versioning/main.tsp +++ b/packages/samples/specs/versioning/main.tsp @@ -3,6 +3,7 @@ import "@typespec/rest"; import "./library.tsp"; using TypeSpec.Versioning; +using TypeSpec.Rest; @service({ title: "Pet Store Service", diff --git a/packages/samples/specs/visibility/visibility.tsp b/packages/samples/specs/visibility/visibility.tsp index 6f0afcb42ef..03f519c3b84 100644 --- a/packages/samples/specs/visibility/visibility.tsp +++ b/packages/samples/specs/visibility/visibility.tsp @@ -2,6 +2,7 @@ import "@typespec/http"; import "@typespec/rest"; using TypeSpec.Http; +using TypeSpec.Rest; @service({ title: "Visibility sample", diff --git a/packages/streams/test/test-host.ts b/packages/streams/test/test-host.ts index 31907bec5ef..1bb51f7ac3a 100644 --- a/packages/streams/test/test-host.ts +++ b/packages/streams/test/test-host.ts @@ -4,7 +4,6 @@ import { StreamsTestLibrary } from "../src/testing/index.js"; export async function createStreamsTestHost() { return createTestHost({ libraries: [StreamsTestLibrary], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } diff --git a/packages/xml/test/test-host.ts b/packages/xml/test/test-host.ts index f0285cd5d73..ada33dbc4c9 100644 --- a/packages/xml/test/test-host.ts +++ b/packages/xml/test/test-host.ts @@ -4,7 +4,6 @@ import { XmlTestLibrary } from "../src/testing/index.js"; export async function createXmlTestHost() { return createTestHost({ libraries: [XmlTestLibrary], - diagnosticFilter: (diag) => diag.severity !== "hint", }); } export async function createXmlTestRunner() { From 55db27b404bf9d551c1e585b14cea12602711edd Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 21:36:47 +0800 Subject: [PATCH 17/22] update tests for unused using --- packages/compiler/src/core/linter.ts | 2 +- packages/compiler/src/core/types.ts | 1 - packages/compiler/src/testing/expect.ts | 2 +- packages/compiler/src/testing/test-host.ts | 16 +- packages/compiler/src/testing/types.ts | 2 - .../test/checker/unused-using.test.ts | 230 +++++++++--------- 6 files changed, 128 insertions(+), 125 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index b74f59e47ba..174ddd15506 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -282,7 +282,7 @@ export function createDefaultLinter(nameResolver: NameResolver): LinterDefinitio severity: "warning", description: "Linter rules for unused using statement.", messages: { - default: paramMessage`${"code"} is declared but never be used.`, + default: paramMessage`'using ${"code"}' is declared but never be used.`, }, create(context) { return { diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index a899d84593e..3ba1a68981a 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -2538,7 +2538,6 @@ export interface DiagnosticDefinition { * Diagnostic severity. * - `warning` - Suppressable, should be used to represent potential issues but not blocking. * - `error` - Non-suppressable, should be used to represent failure to move forward. - * - `hint` - Something to hint to a better way of doing it, like proposing a refactoring. */ readonly severity: "warning" | "error"; /** Messages that can be reported with the diagnostic. */ diff --git a/packages/compiler/src/testing/expect.ts b/packages/compiler/src/testing/expect.ts index c710c6ca422..4732daddf33 100644 --- a/packages/compiler/src/testing/expect.ts +++ b/packages/compiler/src/testing/expect.ts @@ -33,7 +33,7 @@ export interface DiagnosticMatch { /** * Match the severity. */ - severity?: "error" | "warning" | "hint"; + severity?: "error" | "warning"; /** * Name of the file for this diagnostic. diff --git a/packages/compiler/src/testing/test-host.ts b/packages/compiler/src/testing/test-host.ts index 4d31bb841b9..9b38cf08673 100644 --- a/packages/compiler/src/testing/test-host.ts +++ b/packages/compiler/src/testing/test-host.ts @@ -242,7 +242,7 @@ export const StandardTestLibrary: TypeSpecTestLibrary = { }; export async function createTestHost(config: TestHostConfig = {}): Promise { - const testHost = await createTestHostInternal(config); + const testHost = await createTestHostInternal(); await testHost.addTypeSpecLibrary(StandardTestLibrary); if (config.libraries) { for (const library of config.libraries) { @@ -257,7 +257,7 @@ export async function createTestRunner(host?: TestHost): Promise { +async function createTestHostInternal(): Promise { let program: Program | undefined; const libraries: TypeSpecTestLibrary[] = []; const testTypes: Record = {}; @@ -315,10 +315,7 @@ async function createTestHostInternal(config: TestHostConfig): Promise async function compile(main: string, options: CompilerOptions = {}) { const [testTypes, diagnostics] = await compileAndDiagnose(main, options); - const filteredDiagnostics = config.diagnosticFilter - ? diagnostics.filter(config.diagnosticFilter) - : diagnostics; - expectDiagnosticEmpty(filteredDiagnostics); + expectDiagnosticEmpty(diagnostics); return testTypes; } @@ -337,13 +334,10 @@ async function createTestHostInternal(config: TestHostConfig): Promise } const p = await compileProgram(fileSystem.compilerHost, resolveVirtualPath(mainFile), options); program = p; - const filteredDiagnostics = config.diagnosticFilter - ? p.diagnostics.filter(config.diagnosticFilter) - : p.diagnostics; logVerboseTestOutput((log) => - logDiagnostics(filteredDiagnostics, createLogger({ sink: fileSystem.compilerHost.logSink })), + logDiagnostics(p.diagnostics, createLogger({ sink: fileSystem.compilerHost.logSink })), ); - return [testTypes, filteredDiagnostics]; + return [testTypes, p.diagnostics]; } } diff --git a/packages/compiler/src/testing/types.ts b/packages/compiler/src/testing/types.ts index 99204b60608..833c0520974 100644 --- a/packages/compiler/src/testing/types.ts +++ b/packages/compiler/src/testing/types.ts @@ -54,8 +54,6 @@ export interface TypeSpecTestLibrary { export interface TestHostConfig { libraries?: TypeSpecTestLibrary[]; - /** the diagnostics will be ignored if this return false */ - diagnosticFilter?: (diag: Diagnostic) => boolean; } export class TestHostError extends Error { diff --git a/packages/compiler/test/checker/unused-using.test.ts b/packages/compiler/test/checker/unused-using.test.ts index 47db89862f9..b161655a904 100644 --- a/packages/compiler/test/checker/unused-using.test.ts +++ b/packages/compiler/test/checker/unused-using.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, it } from "vitest"; +import { CompilerOptions } from "../../src/index.js"; import { TestHost, createTestHost, @@ -13,6 +14,17 @@ describe("compiler: unused using statements", () => { testHost = await createTestHost(); }); + const diagnoseWithUnusedUsing = async (main: string, options: CompilerOptions = {}) => { + return testHost.diagnose(main, { + ...options, + linterRuleSet: { + enable: { + "@typespec/compiler/unused-using": true, + }, + }, + }); + }; + it("no unused diagnostic when using is used", async () => { testHost.addTypeSpecFile( "main.tsp", @@ -69,12 +81,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N' is declared but never be used.", + severity: "warning", }, ]); }); @@ -105,17 +117,17 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N' is declared but never be used.", + severity: "warning", }, ]); }); @@ -147,17 +159,17 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -189,12 +201,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -222,12 +234,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -247,12 +259,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N' is declared but never be used.", + severity: "warning", }, ]); }); @@ -284,17 +296,17 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N.M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N.M' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using N.M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N.M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -331,17 +343,17 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using TypeSpec.Xyz' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using TypeSpec.Xyz' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Xyz' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Xyz' is declared but never be used.", + severity: "warning", }, ]); }); @@ -367,17 +379,17 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N.A' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N.A' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using M.A' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using M.A' is declared but never be used.", + severity: "warning", }, ]); }); @@ -409,7 +421,7 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnosticEmpty(diagnostics); }); @@ -439,12 +451,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using N.M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using N.M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -464,7 +476,7 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { code: "invalid-ref", @@ -490,7 +502,7 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { code: "duplicate-using", @@ -539,12 +551,12 @@ describe("compiler: unused using statements", () => { } `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using M' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using M' is declared but never be used.", + severity: "warning", }, ]); }); @@ -611,52 +623,52 @@ describe("compiler: unused using statements", () => { model D { a: A3 } `, ); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using Ns1' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2.Ns3' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2.Ns3' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2.Ns3' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2.Ns3' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2.Ns3' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2.Ns3' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1' is declared but never be used.", + severity: "warning", }, { - code: "unused-using", - message: "'using Ns1.Ns2' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Ns1.Ns2' is declared but never be used.", + severity: "warning", }, ]); }); @@ -685,7 +697,7 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnostics(diagnostics, [ { code: "ambiguous-symbol", @@ -712,7 +724,7 @@ describe("compiler: unused using statements", () => { $dec1() {}, }); - const diagnostics = await testHost.diagnose("./"); + const diagnostics = await diagnoseWithUnusedUsing("./"); expectDiagnosticEmpty(diagnostics); }); @@ -726,12 +738,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using TypeSpec' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using TypeSpec' is declared but never be used.", + severity: "warning", }, ]); }); @@ -760,12 +772,12 @@ describe("compiler: unused using statements", () => { } `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using Other' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Other' is declared but never be used.", + severity: "warning", }, ]); }); @@ -803,12 +815,12 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using LibNs' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using LibNs' is declared but never be used.", + severity: "warning", }, ]); }); @@ -846,7 +858,7 @@ describe("compiler: unused using statements", () => { `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnosticEmpty(diagnostics); }); @@ -873,12 +885,12 @@ describe("compiler: unused using statements", () => { } `, ); - const diagnostics = await testHost.diagnose("./", { nostdlib: true }); + const diagnostics = await diagnoseWithUnusedUsing("./", { nostdlib: true }); expectDiagnostics(diagnostics, [ { - code: "unused-using", - message: "'using Other' is declared but never used.", - severity: "hint", + code: "@typespec/compiler/unused-using", + message: "'using Other' is declared but never be used.", + severity: "warning", }, ]); }); From 35c674b7f67ba189c91b827ffd26100bb32aa404 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 21:37:37 +0800 Subject: [PATCH 18/22] remove unnecessary changelot --- .../changes/unused-using-2024-11-27-14-37-20.md | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 .chronus/changes/unused-using-2024-11-27-14-37-20.md diff --git a/.chronus/changes/unused-using-2024-11-27-14-37-20.md b/.chronus/changes/unused-using-2024-11-27-14-37-20.md deleted file mode 100644 index 029d3ae02b3..00000000000 --- a/.chronus/changes/unused-using-2024-11-27-14-37-20.md +++ /dev/null @@ -1,14 +0,0 @@ ---- -changeKind: internal -packages: - - "@typespec/http-specs" - - "@typespec/http" - - "@typespec/openapi" - - "@typespec/openapi3" - - "@typespec/rest" - - "@typespec/streams" - - "@typespec/xml" - - "@typespec/http-server-csharp" ---- - -Fix test for diagnostics of unused using statement \ No newline at end of file From 98c5b81051e38d8dd60a6e94dbda47355962ac51 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 22:27:45 +0800 Subject: [PATCH 19/22] add more test for unused using's logic in lsp --- .../compiler/test/server/unused-using.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 packages/compiler/test/server/unused-using.test.ts diff --git a/packages/compiler/test/server/unused-using.test.ts b/packages/compiler/test/server/unused-using.test.ts new file mode 100644 index 00000000000..8291d83480d --- /dev/null +++ b/packages/compiler/test/server/unused-using.test.ts @@ -0,0 +1,50 @@ +import { strictEqual } from "assert"; +import { describe, it } from "vitest"; +import { DiagnosticSeverity } from "vscode-languageserver"; +import { createTestServerHost } from "../../src/testing/test-server-host.js"; + +describe("compile: server : unused-using diagnostic", () => { + it("hint by default", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 1); + strictEqual(diags[0].code, "@typespec/compiler/unused-using"); + strictEqual(diags[0].severity, DiagnosticSeverity.Hint); + strictEqual(diags[0].message, "'using Foo' is declared but never be used."); + }); + + it("warning per user config", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n enable:\n '@typespec/compiler/unused-using': true", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 1); + strictEqual(diags[0].code, "@typespec/compiler/unused-using"); + strictEqual(diags[0].severity, DiagnosticSeverity.Warning); + strictEqual(diags[0].message, "'using Foo' is declared but never be used."); + }); + + it("nothing per user config", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n enable:\n '@typespec/compiler/unused-using': false", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 0); + }); +}); From 0a22aa3cfbbb9091934eeb36dd4e5d466ccd97b9 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Thu, 13 Feb 2025 23:10:07 +0800 Subject: [PATCH 20/22] refine some code and add more test --- packages/compiler/src/core/linter.ts | 2 +- .../compiler/src/server/compile-service.ts | 9 ++++----- packages/compiler/src/server/serverlib.ts | 8 ++++---- .../compiler/test/server/unused-using.test.ts | 18 ++++++++++++++++-- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index 174ddd15506..a594736d175 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -269,7 +269,7 @@ export function createLinterRuleContext { strictEqual(diags[0].message, "'using Foo' is declared but never be used."); }); - it("warning per user config", async () => { + it("warning if enable === true", async () => { const testHost = await createTestServerHost(); testHost.addOrUpdateDocument( "./tspconfig.yaml", @@ -34,7 +34,7 @@ describe("compile: server : unused-using diagnostic", () => { strictEqual(diags[0].message, "'using Foo' is declared but never be used."); }); - it("nothing per user config", async () => { + it("nothing if enable === false", async () => { const testHost = await createTestServerHost(); testHost.addOrUpdateDocument( "./tspconfig.yaml", @@ -47,4 +47,18 @@ describe("compile: server : unused-using diagnostic", () => { const diags = testHost.getDiagnostics("main.tsp"); strictEqual(diags.length, 0); }); + + it("nothing if disabled", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n disable:\n '@typespec/compiler/unused-using': 'some reason'", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 0); + }); }); From a95875ee2c151242b1b033eceda5295a898ba94e Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Sat, 15 Feb 2025 16:23:45 +0800 Subject: [PATCH 21/22] update linter to expose registerLinterLibrary method --- packages/compiler/src/core/linter.ts | 24 ++++++++++++++-------- packages/compiler/src/core/program.ts | 13 +++++++++--- packages/compiler/test/core/linter.test.ts | 4 +--- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts index a594736d175..710db450205 100644 --- a/packages/compiler/src/core/linter.ts +++ b/packages/compiler/src/core/linter.ts @@ -27,6 +27,7 @@ type LinterLibraryInstance = { linter: LinterResolvedDefinition }; export interface Linter { extendRuleSet(ruleSet: LinterRuleSet): Promise; + registerLinterLibrary(name: string, lib?: LinterLibraryInstance): void; lint(): readonly Diagnostic[]; } @@ -60,7 +61,6 @@ export function resolveLinterDefinition( export function createLinter( program: Program, - nameResolver: NameResolver, loadLibrary: (name: string) => Promise, ): Linter { const tracer = program.tracer.sub("linter"); @@ -68,13 +68,10 @@ export function createLinter( const ruleMap = new Map>(); const enabledRules = new Map>(); const linterLibraries = new Map(); - const builtInLinter: LinterResolvedDefinition = resolveLinterDefinition( - builtInLinterLibraryName, - createDefaultLinter(nameResolver), - ); return { extendRuleSet, + registerLinterLibrary, lint, }; @@ -178,11 +175,13 @@ export function createLinter( return loadedLibrary; } - async function registerLinterLibrary(name: string): Promise { + async function registerLinterLibrary( + name: string, + lib?: LinterLibraryInstance, + ): Promise { tracer.trace("register-library", name); - const library = - name === builtInLinterLibraryName ? { linter: builtInLinter } : await loadLibrary(name); + const library = lib ?? (await loadLibrary(name)); const linter = library?.linter; if (linter?.rules) { for (const rule of linter.rules) { @@ -269,7 +268,14 @@ export function createLinterRuleContext loadLibrary(basedir, name)); + resolver.resolveProgram(); + + const linter = createLinter(program, (name) => loadLibrary(basedir, name)); + linter.registerLinterLibrary(builtInLinterLibraryName, createBuiltInLinterLibrary(resolver)); if (options.linterRuleSet) { program.reportDiagnostics(await linter.extendRuleSet(options.linterRuleSet)); } - resolver.resolveProgram(); program.checker = createChecker(program, resolver); program.checker.checkProgram(); diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts index 8dca9150432..a7d467752b1 100644 --- a/packages/compiler/test/core/linter.test.ts +++ b/packages/compiler/test/core/linter.test.ts @@ -2,7 +2,6 @@ import { describe, it } from "vitest"; import { createLinterRule, createTypeSpecLibrary } from "../../src/core/library.js"; import { Linter, createLinter, resolveLinterDefinition } from "../../src/core/linter.js"; -import { createResolver } from "../../src/core/name-resolver.js"; import type { LibraryInstance, LinterDefinition } from "../../src/index.js"; import { createTestHost, @@ -57,8 +56,7 @@ describe("compiler: linter", () => { await host.compile("main.tsp"); - const resolver = createResolver(host.program); - const linter = createLinter(host.program, resolver, (libName) => + const linter = createLinter(host.program, (libName) => Promise.resolve(libName === "@typespec/test-linter" ? library : undefined), ); return linter; From 872495df3f9edd816330e80a89430b472365b7b6 Mon Sep 17 00:00:00 2001 From: RodgeFu Date: Sat, 15 Feb 2025 16:29:49 +0800 Subject: [PATCH 22/22] update test per feedback --- .../compiler/test/server/unused-using.test.ts | 110 +++++++++--------- 1 file changed, 54 insertions(+), 56 deletions(-) diff --git a/packages/compiler/test/server/unused-using.test.ts b/packages/compiler/test/server/unused-using.test.ts index d42bc76ba38..88203310d9b 100644 --- a/packages/compiler/test/server/unused-using.test.ts +++ b/packages/compiler/test/server/unused-using.test.ts @@ -1,64 +1,62 @@ import { strictEqual } from "assert"; -import { describe, it } from "vitest"; +import { it } from "vitest"; import { DiagnosticSeverity } from "vscode-languageserver"; import { createTestServerHost } from "../../src/testing/test-server-host.js"; -describe("compile: server : unused-using diagnostic", () => { - it("hint by default", async () => { - const testHost = await createTestServerHost(); - testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); - const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); - - await testHost.server.compile(mainFile); - const diags = testHost.getDiagnostics("main.tsp"); - strictEqual(diags.length, 1); - strictEqual(diags[0].code, "@typespec/compiler/unused-using"); - strictEqual(diags[0].severity, DiagnosticSeverity.Hint); - strictEqual(diags[0].message, "'using Foo' is declared but never be used."); - }); - - it("warning if enable === true", async () => { - const testHost = await createTestServerHost(); - testHost.addOrUpdateDocument( - "./tspconfig.yaml", - "linter:\n enable:\n '@typespec/compiler/unused-using': true", - ); - testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); - const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); - - await testHost.server.compile(mainFile); - const diags = testHost.getDiagnostics("main.tsp"); - strictEqual(diags.length, 1); - strictEqual(diags[0].code, "@typespec/compiler/unused-using"); - strictEqual(diags[0].severity, DiagnosticSeverity.Warning); - strictEqual(diags[0].message, "'using Foo' is declared but never be used."); - }); - - it("nothing if enable === false", async () => { - const testHost = await createTestServerHost(); - testHost.addOrUpdateDocument( - "./tspconfig.yaml", - "linter:\n enable:\n '@typespec/compiler/unused-using': false", - ); - testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); - const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); +it("hint by default", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 1); + strictEqual(diags[0].code, "@typespec/compiler/unused-using"); + strictEqual(diags[0].severity, DiagnosticSeverity.Hint); + strictEqual(diags[0].message, "'using Foo' is declared but never be used."); +}); - await testHost.server.compile(mainFile); - const diags = testHost.getDiagnostics("main.tsp"); - strictEqual(diags.length, 0); - }); +it("warning if enable === true", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n enable:\n '@typespec/compiler/unused-using': true", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 1); + strictEqual(diags[0].code, "@typespec/compiler/unused-using"); + strictEqual(diags[0].severity, DiagnosticSeverity.Warning); + strictEqual(diags[0].message, "'using Foo' is declared but never be used."); +}); - it("nothing if disabled", async () => { - const testHost = await createTestServerHost(); - testHost.addOrUpdateDocument( - "./tspconfig.yaml", - "linter:\n disable:\n '@typespec/compiler/unused-using': 'some reason'", - ); - testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); - const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); +it("nothing if enable === false", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n enable:\n '@typespec/compiler/unused-using': false", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 0); +}); - await testHost.server.compile(mainFile); - const diags = testHost.getDiagnostics("main.tsp"); - strictEqual(diags.length, 0); - }); +it("nothing if disabled", async () => { + const testHost = await createTestServerHost(); + testHost.addOrUpdateDocument( + "./tspconfig.yaml", + "linter:\n disable:\n '@typespec/compiler/unused-using': 'some reason'", + ); + testHost.addOrUpdateDocument("./sub.tsp", "namespace Foo; model FooModel {};"); + const mainFile = testHost.addOrUpdateDocument("./main.tsp", 'import "./sub.tsp";\nusing Foo;'); + + await testHost.server.compile(mainFile); + const diags = testHost.getDiagnostics("main.tsp"); + strictEqual(diags.length, 0); });