diff --git a/.chronus/changes/unused-import-namespace-2024-10-18-21-32-11.md b/.chronus/changes/unused-import-namespace-2024-10-18-21-32-11.md new file mode 100644 index 0000000000..6124a6eb65 --- /dev/null +++ b/.chronus/changes/unused-import-namespace-2024-10-18-21-32-11.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Support diagnostics for unused import and using statements diff --git a/.chronus/changes/unused-import-namespace-2024-10-23-11-18-0.md b/.chronus/changes/unused-import-namespace-2024-10-23-11-18-0.md new file mode 100644 index 0000000000..65c572f3fc --- /dev/null +++ b/.chronus/changes/unused-import-namespace-2024-10-23-11-18-0.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/http-specs" +--- + +Remove unnecessary "import" and "using" from tsp code diff --git a/.chronus/changes/unused-import-namespace-2024-10-23-19-12-55.md b/.chronus/changes/unused-import-namespace-2024-10-23-19-12-55.md new file mode 100644 index 0000000000..56def3de19 --- /dev/null +++ b/.chronus/changes/unused-import-namespace-2024-10-23-19-12-55.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@typespec/rest" +--- + +Update test code to handle diagnostics for unnecessary code \ No newline at end of file diff --git a/packages/compiler/src/core/compiler-code-fixes/remove-unnecessary-code.codefix.ts b/packages/compiler/src/core/compiler-code-fixes/remove-unnecessary-code.codefix.ts new file mode 100644 index 0000000000..8fa20b4fea --- /dev/null +++ b/packages/compiler/src/core/compiler-code-fixes/remove-unnecessary-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 f234a97512..eb4033ae22 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 a0b74d2d58..4253b3e7ef 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -576,6 +576,18 @@ const diagnostics = { default: "The #deprecated directive cannot be used more than once on the same declaration.", }, }, + "unused-import": { + severity: "hint", + messages: { + default: paramMessage`Unused import: ${"code"}`, + }, + }, + "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 3641bdb78b..1dc76ad450 100644 --- a/packages/compiler/src/core/name-resolver.ts +++ b/packages/compiler/src/core/name-resolver.ts @@ -60,8 +60,9 @@ 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 { SourceResolution } from "./source-loader.js"; import { AliasStatementNode, AugmentDecoratorStatementNode, @@ -69,8 +70,10 @@ import { EnumStatementNode, Expression, IdentifierNode, + ImportStatementNode, InterfaceStatementNode, IntersectionExpressionNode, + JsSourceFileNode, MemberExpressionNode, ModelExpressionNode, ModelPropertyNode, @@ -79,6 +82,7 @@ import { Node, NodeFlags, NodeLinks, + NoTarget, OperationStatementNode, ProjectionDecoratorReferenceExpressionNode, ProjectionStatementNode, @@ -94,6 +98,7 @@ import { TypeReferenceNode, TypeSpecScriptNode, UnionStatementNode, + UsingStatementNode, } from "./types.js"; export interface NameResolver { @@ -142,6 +147,12 @@ export interface NameResolver { node: TypeReferenceNode | IdentifierNode | MemberExpressionNode, ): ResolutionResult; + /** Get the import statement nodes which is not used in resolving yet */ + getUnusedImports(): ImportStatementNode[]; + + /** Get the using statement nodes which is not used in resolving yet */ + getUnusedUsings(): UsingStatementNode[]; + /** Built-in symbols. */ readonly symbols: { /** Symbol for the global namespace */ @@ -160,7 +171,7 @@ interface ResolveTypReferenceOptions { let currentNodeId = 0; let currentSymbolId = 0; -export function createResolver(program: Program): NameResolver { +export function createResolver(program: Program, sourceResolution: SourceResolution): NameResolver { const mergedSymbols = new Map(); const augmentedSymbolTables = new Map(); const nodeLinks = new Map(); @@ -175,6 +186,19 @@ export function createResolver(program: Program): NameResolver { mutate(globalNamespaceNode).symbol = globalNamespaceSym; mutate(globalNamespaceSym.exports).set(globalNamespaceNode.id.sv, globalNamespaceSym); + /** + * Tracking the nodes whose symbol is resolved through using binding + */ + const nodesWithSymResolvedThroughUsing = new Set(); + /** + * Tracking the nodes whose symbol may reference symbols from other files + */ + const nodesMayRefSymCrossFile = new Set(); + /** + * Tracking the symbols that are used through using. + */ + const usedUsingSym = new Map>(); + const metaTypePrototypes = createMetaTypePrototypes(); const nullSym = createSymbol(undefined, "null", SymbolFlags.None); @@ -222,8 +246,338 @@ export function createResolver(program: Program): NameResolver { resolveTypeReference, getAugmentDecoratorsForSym, + getUnusedImports, + getUnusedUsings, }; + function getUnusedImports(): ImportStatementNode[] { + const notUsed = getNotNeededFileAndLibs(); + if (!notUsed) return []; + + const { notNeededFile, notNeededLib } = notUsed; + const targets = new Set(); + notNeededFile.forEach((file) => { + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type === "project" || (lc.type === "library" && notNeededLib.has(lc.metadata.name))) { + sourceResolution.sourceFileImportedBy.get(file.file.path)?.forEach((target) => { + if ( + target !== NoTarget && + "kind" in target && + target.kind === SyntaxKind.ImportStatement && + target.parent && + program.getSourceFileLocationContext(target.parent.file).type === "project" && + // just ignore the imports in the files which is not needed + !notNeededFile.has(target.parent) + ) { + targets.add(target); + } + }); + } + }); + return [...targets]; + } + + function getNotNeededFileAndLibs() { + const deps = getSourceFileDependencies(); + const { neededFiles, neededLibs } = getNeededFileAndLibsFromFileDependencies(deps); + updateNeededFileForReachability(neededFiles); + + const notNeededFile = new Set(); + const notNeededLib = new Set(); + for (const tspAndJs of [program.sourceFiles.values(), program.jsSourceFiles.values()]) { + for (const file of tspAndJs) { + if (!neededFiles.has(file)) { + notNeededFile.add(file); + } + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type === "library" && !neededLibs.has(lc.metadata.name)) { + notNeededLib.add(lc.metadata.name); + } + } + } + return { notNeededFile, notNeededLib }; + } + + /** + * Some extra files are still needed even when no sym from it is used because some needed file depends on its import to be included in compilation + * i.e. Main.tsp depends on model B in B.tsp, and Main.tsp import A.tsp and A.tsp import B.tsp, then A.tsp is needed too + */ + function updateNeededFileForReachability(neededFile: Set) { + // here we will first go through all the needed files and mark them into 3 categories: reachable, unreachable, unreachable-root + // reachable: the file is reachable from the entrypoint or imported by cli argument/configuration which we don't need to worry about + // unreachable-root: the file is not reachable from other node which we need to do further work to make them reachable + // unreachable: the file is not reachable from reachable node, but it's reachable from unreachable-root directly or indirectly, + // which should be good when we make unreachable-root reachable + // then we will use BFS to make all the unreachable-root reachable with minimal node added. + // please be aware the added node may not be the most optimistic solution with all the unreachable-root node considered together, but it should be good enough + // because we don't want to spend too much resource on this and also we don't expect the graph to be very complex + type Reachability = "reachable" | "unreachable" | "unreachable-root"; + type SourceFile = TypeSpecScriptNode | JsSourceFileNode; + const reachability = new Map(); + markReachability(); + updateForUnreachableRoots(); + return; + + function markReachability() { + neededFile.forEach((file) => { + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type === "project" || lc.type === "library") { + markReachabilityInternal(file); + } + }); + } + + function markReachabilityInternal(file: SourceFile): Reachability | undefined { + if (!neededFile.has(file)) { + return undefined; + } + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type !== "project" && lc.type !== "library") { + // other type's file should be reachable in nature + return "reachable"; + } + let result: Reachability | undefined = reachability.get(file); + if (result) { + return result; + } + + for (const importedFrom of sourceResolution.sourceFileImportedBy.get(file.file.path) ?? []) { + if (importedFrom === NoTarget) { + reachability.set(file, "reachable"); + return "reachable"; + } + const fromFile = importedFrom.parent; + // we only consider needed files when checking reachability + if (fromFile && neededFile.has(fromFile)) { + const fromReachable = markReachabilityInternal(fromFile); + if (fromReachable === "reachable") { + reachability.set(file, "reachable"); + return "reachable"; + } else if (fromReachable !== undefined) { + // cur should be "unreachable" when from is "unreachable" or "unreachable-root" + result = "unreachable"; + } + } + } + if (result === undefined) { + // the file is not imported by any other needed files in the project + result = "unreachable-root"; + } + reachability.set(file, result); + return result; + } + + function updateForUnreachableRoots() { + for (const [file, re] of reachability) { + if (re === "unreachable-root") { + updateForUnreachableRoot(file); + } + } + } + + function updateForUnreachableRoot(file: SourceFile) { + const inQueue = new Set(); + const re = reachability.get(file); + if (re !== "unreachable-root") { + return; + } + type QueueItem = { file: SourceFile; parentIndex: number }; + const queue: QueueItem[] = [{ file, parentIndex: -1 }]; + inQueue.add(file); + let pos = 0; + let cur: QueueItem | undefined = undefined; + while (pos < queue.length) { + cur = queue[pos]; + if (reachability.get(cur.file) === "reachable") { + break; + } else { + const lc = program.getSourceFileLocationContext(cur.file.file); + if (lc.type !== "project" && lc.type !== "library") { + compilerAssert( + false, + "unexpected to reach here. other type file shouldn't be marked as unreachable-root", + ); + } else { + for (const importedFrom of sourceResolution.sourceFileImportedBy.get( + cur.file.file.path, + ) ?? []) { + if (importedFrom === NoTarget) { + break; + } else if (importedFrom.parent && !inQueue.has(importedFrom.parent)) { + inQueue.add(importedFrom.parent); + queue.push({ file: importedFrom.parent, parentIndex: pos }); + } + } + } + } + pos++; + } + if (pos < queue.length) { + while (pos >= 0) { + reachability.set(queue[pos].file, "reachable"); + neededFile.add(queue[pos].file); + pos = queue[pos].parentIndex; + } + } else { + compilerAssert(false, "It's not expected that a file can't be reached"); + } + } + } + + function getNeededFileAndLibsFromFileDependencies( + deps: Map>, + ) { + const neededFiles: Set = new Set(); + const neededLibs: Set = new Set(); + const addNeeded = (file: TypeSpecScriptNode | JsSourceFileNode) => { + if (neededFiles.has(file)) return; + + neededFiles.add(file); + const lc = program.getSourceFileLocationContext(file.file); + if (lc.type === "library") { + neededLibs.add(lc.metadata.name); + } + for (const f of deps.get(file) ?? []) { + addNeeded(f); + } + }; + + program.sourceFiles.forEach((file) => { + const lc = program.getSourceFileLocationContext(file.file); + // entrypoint and import from argument/config is importedBy NoTarget + if ( + lc.type === "project" && + sourceResolution.sourceFileImportedBy.get(file.file.path)?.has(NoTarget) + ) { + addNeeded(file); + } + }); + return { neededFiles, neededLibs }; + } + + /** + * Get the dependencies because + * - node in one file references the sym(node) in another file + * - augment decorator in one file apply to the sym(node) in another file + * Please be aware that "import" is not counted here + */ + function getSourceFileDependencies(): Map< + TypeSpecScriptNode | JsSourceFileNode, + Set + > { + const dependencies = new Map< + TypeSpecScriptNode | JsSourceFileNode, + Set + >(); + + foreachProjectFile(nodesMayRefSymCrossFile, (s, sym) => { + // there may be multiple declarations. i.e. for Decorator/Function, it may have one declaration and one implementation + for (const decl of sym.declarations) { + const t = getSourceFile(decl); + if (!t || s === t) continue; + dependencies.get(s)?.add(t) ?? dependencies.set(s, new Set([t])); + } + }); + + augmentDecoratorsForSym.forEach((decorators, sym) => { + // Add the dependency from the augment decorator target to the augment decorator + sym.declarations.forEach((decl) => { + const s = getSourceFile(decl); + if (!s) return; + decorators.forEach((decorator) => { + const t = getSourceFile(decorator); + if (!t) return; + dependencies.get(s)?.add(t) ?? dependencies.set(s, new Set([t])); + }); + }); + }); + + foreachProjectFile(nodesWithSymResolvedThroughUsing, (s, sym) => { + // for using statement, if it's used somewhere, the dependency should have been handled by the usage. otherwise + // 1. if any of the using target files have already been included in the dependency, then no extra work needed + // 2. if none of the using target files have been included in the dependency + // a. if any of these file is from compiler or synthetic, then no extra work needed + // b. if all of these files are from project or library, then add the file of one declaration into the dependency to make sure + // we won't suggest removing the imports which would cause compiler error for the using statement + // (i.e. for + // import "./a.tsp"; // defines the A namespace + // using A; // which is not used anywhere + // we need to make sure 'import "./a.tsp"' won't be marked as unnecessary because it's needed by 'using A' even when using A is not used anywhere) + // Also please be aware that this is not enough to determine whether a using is needed or not because even there is file dependency, it may be for other reasons + // We still need to compare the symbol in locals to determine the usage, refer to getUnusedUsings() for details + let extraDep: TypeSpecScriptNode | JsSourceFileNode | undefined; + for (const decl of sym.declarations) { + const t = getSourceFile(decl); + if (!t || s === t) continue; + if (dependencies.get(s)?.has(t)) { + return; + } + const tlc = program.getSourceFileLocationContext(t.file); + if (tlc.type === "compiler" || tlc.type === "synthetic") { + return; + } + if (!extraDep) { + extraDep = t; + } + } + if (extraDep) { + dependencies.get(s)?.add(extraDep) ?? dependencies.set(s, new Set([extraDep])); + } + }); + + return dependencies; + + function foreachProjectFile( + nodes: Iterable, + callback: (node: TypeSpecScriptNode | JsSourceFileNode, sym: Sym) => void, + ) { + for (const node of nodes) { + const s = getSourceFile(node); + if (!s) continue; + const lc = program.getSourceFileLocationContext(s.file); + if (lc.type === "project") { + const link = getNodeLinks(node); + const sym = link.resolvedSymbol; + if (sym) { + callback(s, sym); + } + } + } + } + + function getSourceFile(node: Node): TypeSpecScriptNode | JsSourceFileNode | undefined { + return getFirstAncestor( + node, + (n) => n.kind === SyntaxKind.TypeSpecScript || n.kind === SyntaxKind.JsSourceFile, + true, + ) as TypeSpecScriptNode | JsSourceFileNode | undefined; + } + } + + 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) ?? []; } @@ -344,12 +698,35 @@ export function createResolver(program: Program): NameResolver { } else if (node.kind === SyntaxKind.MemberExpression) { return resolveMemberExpression(node, options); } else if (node.kind === SyntaxKind.Identifier) { - return resolveIdentifier(node, options); + const r = resolveIdentifier(node, options); + if (r.resolutionResult & ResolutionResultFlags.Resolved) { + // an IdentifierNode's sym may be resolved through using, namespace or global bindings + // in all case it may reference symbols from other files, so add to track here + // and only consider target node (i.e. 'c' in A.B.c) should be enough + const target = getResolvingTargetNode(node); + nodesMayRefSymCrossFile.add(target); + } + return r; } compilerAssert(false, "Unexpected node kind"); } + /** + * Get the target node that is being resolved which is the source of the reference + */ + function getResolvingTargetNode(id: IdentifierNode): IdentifierNode { + if (id.parent && id.parent.kind === SyntaxKind.MemberExpression) { + let cur = id.parent; + while (cur.parent && cur.parent.kind === SyntaxKind.MemberExpression) { + cur = cur.parent; + } + return cur.id; + } + // for type reference and identifier node, just return the id + return id; + } + function resolveMemberExpression( node: MemberExpressionNode, options: ResolveTypReferenceOptions, @@ -960,6 +1337,16 @@ 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) { + nodesWithSymResolvedThroughUsing.add(node); + 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 +1384,11 @@ export function createResolver(program: Program): NameResolver { []), ]); } + if (usingBinding.flags & SymbolFlags.Using && usingBinding.symbolSource) { + nodesWithSymResolvedThroughUsing.add(node); + 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 ff73599e6d..19a26f6c98 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-unnecessary-code.codefix.js"; import { createSuppressCodeFix } from "./compiler-code-fixes/suppress.codefix.js"; import { compilerAssert } from "./diagnostics.js"; import { resolveTypeSpecEntrypoint } from "./entrypoint-resolution.js"; @@ -26,7 +27,7 @@ import { createTracer } from "./logger/tracer.js"; import { createDiagnostic } from "./messages.js"; import { createResolver } from "./name-resolver.js"; import { CompilerOptions } from "./options.js"; -import { parse, parseStandaloneTypeReference } from "./parser.js"; +import { parse, parseStandaloneTypeReference, visitChildren } from "./parser.js"; import { getDirectoryPath, joinPaths, resolvePath } from "./path-utils.js"; import { createProjector } from "./projector.js"; import { @@ -45,11 +46,13 @@ import { EmitContext, EmitterFunc, Entity, + IdentifierNode, JsSourceFileNode, LibraryInstance, LibraryMetadata, LiteralType, LocationContext, + MemberExpressionNode, ModuleLibraryMetadata, Namespace, NoTarget, @@ -202,7 +205,7 @@ export async function compile( const basedir = getDirectoryPath(resolvedMain) || "/"; await checkForCompilerVersionMismatch(basedir); - await loadSources(resolvedMain); + const srcLoader = await loadSources(resolvedMain); let emit = options.emit; let emitterOptions = options.options; @@ -232,7 +235,7 @@ export async function compile( program.reportDiagnostics(await linter.extendRuleSet(options.linterRuleSet)); } - const resolver = createResolver(program); + const resolver = createResolver(program, srcLoader.resolution); resolver.resolveProgram(); program.checker = createChecker(program, resolver); program.checker.checkProgram(); @@ -244,6 +247,7 @@ export async function compile( await runValidators(); validateRequiredImports(); + validateUnusedCode(); await validateLoadedLibraries(); if (!continueToNextStage) { @@ -260,6 +264,67 @@ export async function compile( return program; + function isProjectionUsed() { + const isProjectionStatement = (node: Node): true | undefined => { + if (node.kind === SyntaxKind.ProjectionStatement) { + return true; + } + return visitChildren(node, isProjectionStatement); + }; + for (const file of program.sourceFiles.values()) { + if (program.getSourceFileLocationContext(file.file).type === "project") { + if (visitChildren(file, isProjectionStatement) === true) { + return true; + } + } + } + return false; + } + + function validateUnusedCode() { + // Don't provide unused diagnostics if customer is using projection in the project because + // the projection statements will only be processed when applying projection. There is no way to determine + // whether "import" or "using" is referenced from them, so we just skip here to avoid providing incorrect suggestions (diagnostics) + // This should be fine for now considering projection is an experiemental feature. + if (isProjectionUsed()) return; + + resolver.getUnusedImports().forEach((target) => { + if (!requireImports.has(target.path.value)) { + reportDiagnostic( + createDiagnostic({ + code: "unused-import", + target: target, + format: { + code: `import "${target.path.value}"`, + }, + codefixes: [removeUnusedCodeCodeFix(target)], + }), + ); + } + }); + + 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. */ @@ -350,6 +415,8 @@ export async function compile( binder.bindJsSourceFile(jsFile); } program.reportDiagnostics(sourceResolution.diagnostics); + + return sourceLoader; } async function loadIntrinsicTypes(loader: SourceLoader) { diff --git a/packages/compiler/src/core/source-loader.ts b/packages/compiler/src/core/source-loader.ts index 16fdcc60f1..cdb68cf54b 100644 --- a/packages/compiler/src/core/source-loader.ts +++ b/packages/compiler/src/core/source-loader.ts @@ -15,6 +15,7 @@ import { getDirectoryPath } from "./path-utils.js"; import { createSourceFile } from "./source-file.js"; import { DiagnosticTarget, + ImportStatementNode, ModuleLibraryMetadata, NodeFlags, NoTarget, @@ -36,6 +37,8 @@ export interface SourceResolution { /** Javascript source files(Entrypoint only) */ readonly jsSourceFiles: Map; + /** How source files are imported. NoTarget means the file is entrypoint or imported from cli/configuration directly */ + readonly sourceFileImportedBy: Map>; readonly locationContexts: WeakMap; readonly loadedLibraries: Map; @@ -85,6 +88,7 @@ export async function createSourceLoader( const sourceFiles = new Map(); const jsSourceFiles = new Map(); const loadedLibraries = new Map(); + const sourceFileImportedBy = new Map>(); async function importFile( path: string, @@ -117,6 +121,7 @@ export async function createSourceLoader( resolution: { sourceFiles, jsSourceFiles, + sourceFileImportedBy, locationContexts: sourceFileLocationContexts, loadedLibraries: loadedLibraries, diagnostics: diagnostics.diagnostics, @@ -129,6 +134,10 @@ export async function createSourceLoader( diagnosticTarget: DiagnosticTarget | typeof NoTarget, ) { if (seenSourceFiles.has(path)) { + const file = sourceFiles.get(path); + if (file) { + updateImportedBy(file, diagnosticTarget); + } return; } seenSourceFiles.add(path); @@ -139,7 +148,8 @@ export async function createSourceLoader( if (file) { sourceFileLocationContexts.set(file, locationContext); - await loadTypeSpecScript(file); + const tss = await loadTypeSpecScript(file); + updateImportedBy(tss, diagnosticTarget); } } @@ -287,17 +297,29 @@ export async function createSourceLoader( ) { const sourceFile = jsSourceFiles.get(path); if (sourceFile !== undefined) { + updateImportedBy(sourceFile, diagnosticTarget); return sourceFile; } const file = diagnostics.pipe(await loadJsFile(host, path, diagnosticTarget)); if (file !== undefined) { sourceFileLocationContexts.set(file.file, locationContext); + updateImportedBy(file, diagnosticTarget); jsSourceFiles.set(path, file); } return file; } + function updateImportedBy( + file: JsSourceFileNode | TypeSpecScriptNode, + target: DiagnosticTarget | typeof NoTarget, + ) { + if (target === NoTarget || ("kind" in target && target.kind === SyntaxKind.ImportStatement)) { + sourceFileImportedBy.get(file.file.path)?.add(target) ?? + sourceFileImportedBy.set(file.file.path, new Set([target])); + } + } + function getResolveModuleHost(): ResolveModuleHost { return { realpath: host.realpath, diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index ec31817968..93803cbdb7 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 f9dbe4718a..5c5f687676 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 eb47f25d09..f5c906f240 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-import" || 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 4732daddf3..c710c6ca42 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/src/testing/test-host.ts b/packages/compiler/src/testing/test-host.ts index 9b38cf0867..c20b718584 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 = {}; @@ -300,6 +300,7 @@ async function createTestHostInternal(): Promise { await fileSystem.addTypeSpecLibrary(lib); }, compile, + compileWithProgram, diagnose, compileAndDiagnose, testTypes, @@ -313,15 +314,38 @@ async function createTestHostInternal(): Promise { }, }; + function filterUnusedDiagnostics(diagnostics: readonly Diagnostic[]) { + if (config.checkUnnecessaryDiagnostics === true) { + return diagnostics; + } else { + // don't check hint diagnostics by default considering many test case contains unnecessary using or import + // in test tsp code which actually doesn't matter to the test + return diagnostics.filter((d) => d.code !== "unused-import" && d.code !== "unused-using"); + } + } + async function compile(main: string, options: CompilerOptions = {}) { const [testTypes, diagnostics] = await compileAndDiagnose(main, options); - expectDiagnosticEmpty(diagnostics); + expectDiagnosticEmpty(filterUnusedDiagnostics(diagnostics)); return testTypes; } + async function compileWithProgram( + mainFile: string, + options?: CompilerOptions, + oldProgram?: Program, + ) { + return compileProgram( + fileSystem.compilerHost, + resolveVirtualPath(mainFile), + options, + oldProgram, + ); + } + async function diagnose(main: string, options: CompilerOptions = {}) { const [, diagnostics] = await compileAndDiagnose(main, options); - return diagnostics; + return filterUnusedDiagnostics(diagnostics); } async function compileAndDiagnose( @@ -337,7 +361,7 @@ async function createTestHostInternal(): Promise { logVerboseTestOutput((log) => logDiagnostics(p.diagnostics, createLogger({ sink: fileSystem.compilerHost.logSink })), ); - return [testTypes, p.diagnostics]; + return [testTypes, filterUnusedDiagnostics(p.diagnostics)]; } } diff --git a/packages/compiler/src/testing/types.ts b/packages/compiler/src/testing/types.ts index 833c052097..054d6da9cd 100644 --- a/packages/compiler/src/testing/types.ts +++ b/packages/compiler/src/testing/types.ts @@ -19,6 +19,11 @@ export interface TestHost extends TestFileSystem { testTypes: Record; compile(main: string, options?: CompilerOptions): Promise>; + compileWithProgram( + main: string, + options?: CompilerOptions, + oldProgram?: Program, + ): Promise; diagnose(main: string, options?: CompilerOptions): Promise; compileAndDiagnose( main: string, @@ -54,6 +59,7 @@ export interface TypeSpecTestLibrary { export interface TestHostConfig { libraries?: TypeSpecTestLibrary[]; + checkUnnecessaryDiagnostics?: boolean; } export class TestHostError extends Error { diff --git a/packages/compiler/test/checker/alias.test.ts b/packages/compiler/test/checker/alias.test.ts index cef3c4df52..9ca0b05f0a 100644 --- a/packages/compiler/test/checker/alias.test.ts +++ b/packages/compiler/test/checker/alias.test.ts @@ -12,7 +12,7 @@ describe("compiler: aliases", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function getOptionAtIndex(union: Union, index: number): Type { diff --git a/packages/compiler/test/checker/augment-decorators.test.ts b/packages/compiler/test/checker/augment-decorators.test.ts index 19ff624a17..b3fd5f762f 100644 --- a/packages/compiler/test/checker/augment-decorators.test.ts +++ b/packages/compiler/test/checker/augment-decorators.test.ts @@ -11,7 +11,7 @@ import { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("run decorator without arguments", async () => { @@ -337,10 +337,12 @@ describe("emit diagnostic", () => { model Foo {} @@notDefined(Foo, "A string Foo"); `); - expectDiagnostics(diagnostics, { - code: "invalid-ref", - message: "Unknown decorator @notDefined", - }); + expectDiagnostics(diagnostics, [ + { + code: "invalid-ref", + message: "Unknown decorator @notDefined", + }, + ]); }); it("if target is invalid identifier", async () => { diff --git a/packages/compiler/test/checker/check-parse-errors.test.ts b/packages/compiler/test/checker/check-parse-errors.test.ts index c9845ab95b..7fc9680a56 100644 --- a/packages/compiler/test/checker/check-parse-errors.test.ts +++ b/packages/compiler/test/checker/check-parse-errors.test.ts @@ -5,7 +5,7 @@ describe("compiler: semantic checks on source with parse errors", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("reports semantic errors in addition to parse errors", async () => { diff --git a/packages/compiler/test/checker/clone-type.test.ts b/packages/compiler/test/checker/clone-type.test.ts index fed5fac03e..e62b9b663b 100644 --- a/packages/compiler/test/checker/clone-type.test.ts +++ b/packages/compiler/test/checker/clone-type.test.ts @@ -10,7 +10,7 @@ describe("compiler: type cloning", () => { const blues = new Set(); beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("test.js", { $blue(_: Program, t: Type) { blues.add(t); diff --git a/packages/compiler/test/checker/decorators.test.ts b/packages/compiler/test/checker/decorators.test.ts index 41e05c83f1..240b007c8f 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, @@ -23,7 +24,7 @@ describe("compiler: checker: decorators", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); describe("declaration", () => { @@ -42,6 +43,23 @@ describe("compiler: checker: decorators", () => { }); }); + const unnecessaryDiags: DiagnosticMatch[] = [ + { + code: "unused-import", + message: `Unused import: import "./test.js"`, + severity: "hint", + }, + { + 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 +75,7 @@ describe("compiler: checker: decorators", () => { }); it("defined at root", async () => { - await runner.compile(` + await compileAndCheckDiags(` extern dec otherDec(target: unknown); `); @@ -67,7 +85,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 +104,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 +114,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 +134,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 +184,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 +212,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 +224,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 +236,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 +248,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 +267,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 +284,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 +301,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 +317,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 +349,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 +381,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 +400,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 +574,12 @@ describe("compiler: checker: decorators", () => { @test model Foo {} `); - expectDiagnosticEmpty(diagnostics.filter((x) => x.code !== "deprecated")); + expectDiagnosticEmpty( + diagnostics.filter( + (x) => + x.code !== "deprecated" && x.code !== "unused-import" && x.code !== "unused-using", + ), + ); return calledArgs![2]; } diff --git a/packages/compiler/test/checker/deprecation.test.ts b/packages/compiler/test/checker/deprecation.test.ts index f03ffc0d24..239d32bf8e 100644 --- a/packages/compiler/test/checker/deprecation.test.ts +++ b/packages/compiler/test/checker/deprecation.test.ts @@ -191,7 +191,7 @@ describe("compiler: checker: deprecation", () => { }); it("emits deprecation for use of deprecated decorator signatures", async () => { - const testHost: TestHost = await createTestHost(); + const testHost: TestHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("test.js", { $testDec: () => {} }); const runner = createTestWrapper(testHost); diff --git a/packages/compiler/test/checker/duplicate-ids.test.ts b/packages/compiler/test/checker/duplicate-ids.test.ts index cca3297c1a..074d447190 100644 --- a/packages/compiler/test/checker/duplicate-ids.test.ts +++ b/packages/compiler/test/checker/duplicate-ids.test.ts @@ -6,7 +6,7 @@ describe("compiler: duplicate declarations", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("reports duplicate template parameters", async () => { @@ -72,6 +72,7 @@ describe("compiler: duplicate declarations", () => { ` import "./a.tsp"; import "./b.tsp"; + alias A = N.A; `, ); testHost.addTypeSpecFile( @@ -131,7 +132,7 @@ describe("compiler: duplicate declarations", () => { "main.tsp", ` import "./a.tsp"; - import "./b.tsp"; + import "./b.tsp"; `, ); testHost.addTypeSpecFile("a.tsp", "namespace N {}"); diff --git a/packages/compiler/test/checker/effective-type.test.ts b/packages/compiler/test/checker/effective-type.test.ts index 480fd93ec0..2cba7ed8dd 100644 --- a/packages/compiler/test/checker/effective-type.test.ts +++ b/packages/compiler/test/checker/effective-type.test.ts @@ -10,7 +10,7 @@ describe("compiler: effective type", () => { beforeEach(async () => { const removeSymbol = Symbol("remove"); - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("remove.js", { $remove: ({ program }: DecoratorContext, entity: Type) => { program.stateSet(removeSymbol).add(entity); diff --git a/packages/compiler/test/checker/enum.test.ts b/packages/compiler/test/checker/enum.test.ts index 34a3910743..3e57ac3ed4 100644 --- a/packages/compiler/test/checker/enum.test.ts +++ b/packages/compiler/test/checker/enum.test.ts @@ -8,7 +8,7 @@ describe("compiler: enums", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("can be valueless", async () => { diff --git a/packages/compiler/test/checker/global-namespace.test.ts b/packages/compiler/test/checker/global-namespace.test.ts index 1933f52af4..211ce47651 100644 --- a/packages/compiler/test/checker/global-namespace.test.ts +++ b/packages/compiler/test/checker/global-namespace.test.ts @@ -1,13 +1,13 @@ import assert, { notStrictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { Model } from "../../src/core/types.js"; -import { TestHost, createTestHost } from "../../src/testing/index.js"; +import { TestHost, createTestHost, expectDiagnostics } from "../../src/testing/index.js"; describe("compiler: global namespace", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); describe("it adds top level entities to the global namespace", () => { @@ -63,7 +63,14 @@ describe("compiler: global namespace", () => { it("adds top-level namespaces", async () => { testHost.addTypeSpecFile("a.tsp", `namespace Foo {}`); - await testHost.compile("./"); + const [_, diags] = await testHost.compileAndDiagnose("./"); + expectDiagnostics(diags, [ + { + code: "unused-import", + message: `Unused import: import "./a.tsp"`, + severity: "hint", + }, + ]); const globalNamespaceType = testHost.program.checker.getGlobalNamespaceType(); assert( @@ -79,7 +86,14 @@ describe("compiler: global namespace", () => { it("adds top-level models", async () => { testHost.addTypeSpecFile("a.tsp", `model MyModel {}`); - await testHost.compile("./"); + const [_, diags] = await testHost.compileAndDiagnose("./"); + expectDiagnostics(diags, [ + { + code: "unused-import", + message: `Unused import: import "./a.tsp"`, + severity: "hint", + }, + ]); const globalNamespaceType = testHost.program.checker.getGlobalNamespaceType(); assert( @@ -91,7 +105,14 @@ describe("compiler: global namespace", () => { it("adds top-level operations", async () => { testHost.addTypeSpecFile("a.tsp", `op myOperation(): string;`); - await testHost.compile("./"); + const [_, diags] = await testHost.compileAndDiagnose("./"); + expectDiagnostics(diags, [ + { + code: "unused-import", + message: `Unused import: import "./a.tsp"`, + severity: "hint", + }, + ]); const globalNamespaceType = testHost.program.checker.getGlobalNamespaceType(); assert( diff --git a/packages/compiler/test/checker/imports.test.ts b/packages/compiler/test/checker/imports.test.ts index 12f52d53de..55cc9a4251 100644 --- a/packages/compiler/test/checker/imports.test.ts +++ b/packages/compiler/test/checker/imports.test.ts @@ -7,6 +7,7 @@ import { ProjectLocationContext, } from "../../src/core/index.js"; import { + DiagnosticMatch, TestHost, createTestHost, expectDiagnostics, @@ -18,7 +19,7 @@ describe("compiler: imports", () => { let host: TestHost; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function expectFileLoaded(files: { typespec?: string[]; js?: string[] }) { @@ -222,18 +223,35 @@ describe("compiler: imports", () => { function givenStructure(config: ScopeTest): ScopeExpectation { return { expectScopes: async (scopes: Record) => { + const unnecessaryImportDiags: DiagnosticMatch[] = []; for (const [filename, fileConfig] of Object.entries(config.structure)) { if (filename.endsWith(".tsp")) { host.addTypeSpecFile( filename, (fileConfig as string[]).map((x) => `import "${x}";`).join("\n"), ); + if (!filename.includes("my-lib")) { + (fileConfig as string[]).forEach((x) => { + unnecessaryImportDiags.push({ + code: "unused-import", + message: `Unused import: import "${x}"`, + severity: "hint", + }); + }); + } } else { host.addTypeSpecFile(filename, JSON.stringify(fileConfig, null, 2)); } } - await host.compile(config.entrypoint); + if (unnecessaryImportDiags.length === 0) { + await host.compile(config.entrypoint); + } else { + const [_, diags] = await host.compileAndDiagnose(config.entrypoint); + const cmpFunc = (a: { message?: string | RegExp }, b: { message?: string | RegExp }) => + (a.message ?? "") < (b.message ?? "") ? -1 : 1; + expectDiagnostics([...diags].sort(cmpFunc), unnecessaryImportDiags.sort(cmpFunc)); + } for (const [filename, expectedScope] of Object.entries(scopes)) { const file = host.program.sourceFiles.get(resolveVirtualPath(filename)); ok(file, `Expected to have file "${filename}"`); diff --git a/packages/compiler/test/checker/interface.test.ts b/packages/compiler/test/checker/interface.test.ts index f1a20f0f05..932d82c557 100644 --- a/packages/compiler/test/checker/interface.test.ts +++ b/packages/compiler/test/checker/interface.test.ts @@ -16,7 +16,7 @@ describe("compiler: interfaces", () => { let runner: BasicTestRunner; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = await createTestRunner(testHost); }); diff --git a/packages/compiler/test/checker/intersections.test.ts b/packages/compiler/test/checker/intersections.test.ts index 7210f2ccf1..24ae99a759 100644 --- a/packages/compiler/test/checker/intersections.test.ts +++ b/packages/compiler/test/checker/intersections.test.ts @@ -13,7 +13,7 @@ describe("compiler: intersections", () => { let runner: BasicTestRunner; beforeEach(async () => { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host); }); diff --git a/packages/compiler/test/checker/model-circular-references.test.ts b/packages/compiler/test/checker/model-circular-references.test.ts index ad5a1e5907..ad13cda499 100644 --- a/packages/compiler/test/checker/model-circular-references.test.ts +++ b/packages/compiler/test/checker/model-circular-references.test.ts @@ -11,7 +11,7 @@ describe("compiler: model circular references", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("model can reference itself", async () => { diff --git a/packages/compiler/test/checker/model.test.ts b/packages/compiler/test/checker/model.test.ts index 020d97f078..4bb7bf4430 100644 --- a/packages/compiler/test/checker/model.test.ts +++ b/packages/compiler/test/checker/model.test.ts @@ -21,7 +21,7 @@ describe("compiler: models", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("allow template parameters passed into decorators", async () => { @@ -794,7 +794,7 @@ describe("compiler: models", () => { const blues = new WeakSet(); const reds = new WeakSet(); beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("dec.js", { $blue(p: any, t: Type) { blues.add(t); @@ -862,7 +862,6 @@ describe("compiler: models", () => { testHost.addTypeSpecFile( "main.tsp", ` - import "./dec.js"; @test model A { x: int32 } model B extends A { y: string }; @test model C is B { } @@ -877,7 +876,6 @@ describe("compiler: models", () => { testHost.addTypeSpecFile( "main.tsp", ` - import "./dec.js"; @test model A is string[]; `, ); @@ -889,7 +887,6 @@ describe("compiler: models", () => { testHost.addTypeSpecFile( "main.tsp", ` - import "./dec.js"; @test model A is (string | int32)[]; `, ); @@ -934,7 +931,6 @@ describe("compiler: models", () => { testHost.addTypeSpecFile( "main.tsp", ` - import "./dec.js"; model A { x: int32 } model B is A { x: int32 }; `, diff --git a/packages/compiler/test/checker/namespaces.test.ts b/packages/compiler/test/checker/namespaces.test.ts index 90540a1f50..f6030bb807 100644 --- a/packages/compiler/test/checker/namespaces.test.ts +++ b/packages/compiler/test/checker/namespaces.test.ts @@ -19,7 +19,7 @@ describe("compiler: namespaces with blocks", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("blue.js", { $blue }); }); @@ -91,6 +91,9 @@ describe("compiler: namespaces with blocks", () => { import "./a.tsp"; import "./b.tsp"; import "./c.tsp"; + + using N; + alias A = X | Y | Z; `, ); testHost.addTypeSpecFile( @@ -131,6 +134,9 @@ describe("compiler: namespaces with blocks", () => { import "./a.tsp"; import "./b.tsp"; import "./c.tsp"; + model M { + ...N.Z; + } `, ); testHost.addTypeSpecFile( @@ -276,6 +282,8 @@ describe("compiler: namespaces with blocks", () => { import "./a.tsp"; import "./b.tsp"; import "./c.tsp"; + + alias fooOp = foo.foo; `, ); testHost.addTypeSpecFile( @@ -349,7 +357,7 @@ describe("compiler: blockless namespaces", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("blue.js", { $blue }); }); @@ -360,6 +368,7 @@ describe("compiler: blockless namespaces", () => { import "./a.tsp"; import "./b.tsp"; import "./c.tsp"; + alias foo = Z; `, ); testHost.addTypeSpecFile( @@ -482,6 +491,7 @@ describe("compiler: blockless namespaces", () => { ` import "./a.tsp"; import "./b.tsp"; + alias foo = X; `, ); testHost.addTypeSpecFile( @@ -556,7 +566,7 @@ describe("compiler: namespace type name", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("prefix with the namespace of the entity", async () => { @@ -614,7 +624,7 @@ describe("compiler: decorators in namespaces", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("puts decorators in namespaces using an exported string", async () => { diff --git a/packages/compiler/test/checker/operations.test.ts b/packages/compiler/test/checker/operations.test.ts index 60cea11379..27e647f191 100644 --- a/packages/compiler/test/checker/operations.test.ts +++ b/packages/compiler/test/checker/operations.test.ts @@ -8,7 +8,7 @@ describe("compiler: operations", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("can return void", async () => { diff --git a/packages/compiler/test/checker/references.test.ts b/packages/compiler/test/checker/references.test.ts index 1d6a58b795..a7991f5381 100644 --- a/packages/compiler/test/checker/references.test.ts +++ b/packages/compiler/test/checker/references.test.ts @@ -12,7 +12,7 @@ import { describe("compiler: references", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function itCanReference({ diff --git a/packages/compiler/test/checker/relation.test.ts b/packages/compiler/test/checker/relation.test.ts index 7f1ba4a3aa..92a5be106b 100644 --- a/packages/compiler/test/checker/relation.test.ts +++ b/packages/compiler/test/checker/relation.test.ts @@ -28,7 +28,7 @@ interface RelatedTypeOptions { let runner: BasicTestRunner; let host: TestHost; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host); }); @@ -47,7 +47,14 @@ describe("compiler: checker: type relations", () => { ${commonCode ?? ""} extern dec mock(target: unknown, source: ┆${source}, value: ${target}); `); - await runner.compile(code); + const [_, diags] = await runner.compileAndDiagnose(code); + expectDiagnostics(diags, [ + { + code: "unused-import", + message: `Unused import: import "./mock.js"`, + severity: "hint", + }, + ]); const decDeclaration = runner.program .getGlobalNamespaceType() .decoratorDeclarations.get("mock"); diff --git a/packages/compiler/test/checker/resolve-type-reference.test.ts b/packages/compiler/test/checker/resolve-type-reference.test.ts index eac9e00200..1d407b66b2 100644 --- a/packages/compiler/test/checker/resolve-type-reference.test.ts +++ b/packages/compiler/test/checker/resolve-type-reference.test.ts @@ -11,7 +11,7 @@ import { describe("compiler: resolveTypeReference", () => { let runner: BasicTestRunner; beforeEach(async () => { - runner = createTestWrapper(await createTestHost()); + runner = createTestWrapper(await createTestHost({ checkUnnecessaryDiagnostics: true })); }); async function expectResolve(reference: string, code: string) { diff --git a/packages/compiler/test/checker/scalar.test.ts b/packages/compiler/test/checker/scalar.test.ts index b97b15c335..178bbc265b 100644 --- a/packages/compiler/test/checker/scalar.test.ts +++ b/packages/compiler/test/checker/scalar.test.ts @@ -13,7 +13,7 @@ describe("compiler: scalars", () => { let runner: BasicTestRunner; beforeEach(async () => { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host); }); diff --git a/packages/compiler/test/checker/spread.test.ts b/packages/compiler/test/checker/spread.test.ts index bb3c1ee0df..3e8cd47c9b 100644 --- a/packages/compiler/test/checker/spread.test.ts +++ b/packages/compiler/test/checker/spread.test.ts @@ -18,7 +18,7 @@ describe("compiler: spread", () => { let runner: BasicTestRunner; beforeEach(async () => { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); host.addJsFile("blue.js", { $blue }); runner = createTestWrapper(host); }); diff --git a/packages/compiler/test/checker/templates.test.ts b/packages/compiler/test/checker/templates.test.ts index 59ccf98f43..06ce9d5f84 100644 --- a/packages/compiler/test/checker/templates.test.ts +++ b/packages/compiler/test/checker/templates.test.ts @@ -18,7 +18,7 @@ describe("compiler: templates", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function getLineAndCharOfDiagnostic(diagnostic: Diagnostic) { diff --git a/packages/compiler/test/checker/typeof.test.ts b/packages/compiler/test/checker/typeof.test.ts index c4ebe9a9b1..f1300967b9 100644 --- a/packages/compiler/test/checker/typeof.test.ts +++ b/packages/compiler/test/checker/typeof.test.ts @@ -80,7 +80,7 @@ describe("typeof can be used to force sending a type to a decorator that accept beforeEach(async () => { called = undefined; - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); host.addJsFile("dec.js", { $foo: (_ctx: any, _target: any, value: any) => { called = value; diff --git a/packages/compiler/test/checker/union.test.ts b/packages/compiler/test/checker/union.test.ts index 7b59070081..3a2b99c44c 100644 --- a/packages/compiler/test/checker/union.test.ts +++ b/packages/compiler/test/checker/union.test.ts @@ -7,7 +7,7 @@ describe("compiler: union declarations", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("can be declared and decorated", async () => { diff --git a/packages/compiler/test/checker/unused-imports.test.ts b/packages/compiler/test/checker/unused-imports.test.ts new file mode 100644 index 0000000000..8385be7a56 --- /dev/null +++ b/packages/compiler/test/checker/unused-imports.test.ts @@ -0,0 +1,1039 @@ +import { beforeEach, describe, it } from "vitest"; +import { createTypeSpecLibrary } from "../../src/index.js"; +import { + createTestHost, + expectDiagnosticEmpty, + expectDiagnostics, + TestHost, +} from "../../src/testing/index.js"; + +describe("compiler: unused imports", () => { + let host: TestHost; + + beforeEach(async () => { + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); + }); + + it("no unused diagnostic for import file with usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + model A extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic for import file without usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./b.tsp"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic for import file with reference from template", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + model A { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused diagnostic for import JS file with decorator usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./blue.js"; + + @blue + model A {} + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic for import JS file without usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./blue.js"; + + model A {} + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./blue.js"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic for import file with indirect usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + model A extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B extends C{ } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused diagnostic for multiple import file with usage", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + import "./blue.js"; + model A extends B { } + @blue + model A2 extends C { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic for C when Main->B, C->B", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + model A extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C extends B { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + ]); + }); + + it("unused diagnostic for C when Main->B, C->D", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + import "./d.tsp"; + model A extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C extends D { }; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + model D { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + { + code: "unused-import", + message: `Unused import: import "./d.tsp"`, + severity: "hint", + }, + ]); + }); + + it("unused diagnostic for import directory without usage", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "./test"; + + model A { x: int16 } + `, + ); + host.addTypeSpecFile( + "test/main.tsp", + ` + model C { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./test"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic for import directory with usage & unused import in lib", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "./test"; + + model A extends DirA { x: C } + `, + ); + host.addTypeSpecFile( + "test/main.tsp", + ` + import "./dir-a.tsp"; + model C { } + `, + ); + host.addTypeSpecFile( + "test/dir-a.tsp", + ` + model DirA { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused import library", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + + model A { x: int16 } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + host.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + model C { } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "my-lib"`, + severity: "hint", + }, + ]); + }); + + it("unused import library but required by emitter", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + + model A { x: int16 } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + host.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + model C { } + `, + ); + host.addTypeSpecFile( + "node_modules/fake-emitter/package.json", + JSON.stringify({ + main: "index.js", + }), + ); + const fakeEmitter = createTypeSpecLibrary({ + name: "fake-emitter", + diagnostics: {}, + requireImports: ["my-lib"], + emitter: { + options: { + type: "object", + properties: { + "asset-dir": { type: "string", format: "absolute-path", nullable: true }, + "max-files": { type: "number", nullable: true }, + }, + additionalProperties: false, + }, + }, + }); + host.addJsFile("node_modules/fake-emitter/index.js", { + $lib: fakeEmitter, + $onEmit: () => {}, + }); + + const diagnostics = await host.diagnose("main.tsp", { + emit: ["fake-emitter"], + }); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused import library when there is one ref", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + + model B { x: LibA.A } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + host.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + import "./lib-a.tsp"; + using LibA; + model C extends A { } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/lib-a.tsp", + ` + namespace LibA; + model A { x: int16 } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused import library when there is one using even when it's not used", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + using LibA; + model B { x: int32 } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + host.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + import "./lib-a.tsp"; + using LibA; + model C extends A { } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/lib-a.tsp", + ` + namespace LibA; + model A { x: int16 } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-using", + message: `Unused using: using LibA`, + severity: "hint", + }, + ]); + }); + + it("no unused import library when there is one ref through using", async () => { + host.addTypeSpecFile( + "main.tsp", + ` + import "my-lib"; + using LibA; + model B { x: A } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/package.json", + JSON.stringify({ + name: "my-test-lib", + exports: { ".": { typespec: "./main.tsp" } }, + }), + ); + host.addTypeSpecFile( + "node_modules/my-lib/main.tsp", + ` + import "./lib-a.tsp"; + using LibA; + model C { } + `, + ); + host.addTypeSpecFile( + "node_modules/my-lib/lib-a.tsp", + ` + namespace LibA; + model A { x: int16 } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic for multiple import to one file", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + import "./blue.js"; + @blue + model A extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + import "./blue.js"; + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + @blue + model C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic Main -> B & C & D, B -> C, D ref C", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + import "./d.tsp"; + import "./blue.js"; + @blue + model A extends D { } + model AA extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + import "./blue.js"; + + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + @blue + model C { }; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + model D extends C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic Main -> B & C & D, B -> C, D ref C, Main no-ref D", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + import "./d.tsp"; + import "./blue.js"; + @blue + model A extends B { } + model AA extends B { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + import "./blue.js"; + + model B { } + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + @blue + model C { }; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + model D extends C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + { + code: "unused-import", + message: `Unused import: import "./d.tsp"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic Main -> B -> C -> D, Main ref D", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + model AA extends D { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + import "./d.tsp"; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + model D { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("unused diagnostic Main -> B -> C -> D and Main -> C, Main ref D", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + model AA extends D { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + import "./d.tsp"; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + model D { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./b.tsp"`, + severity: "hint", + }, + ]); + }); + + it("no unused diagnostic Main -> B & C, Main ref C through Alias in B", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + model AA extends AliasC { } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + alias AliasC = C; + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C { }; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused diagnostic when referenced in same namespace", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + namespace N { + model M extends B {} + } + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + namespace N { + model A {} + } + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + namespace N { + model B extends A {} + } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused diagnostic when referenced in same namespace (blockless)", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + namespace N; + model M extends B {} + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model A {}; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + namespace N; + model B extends A {} + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("no unused diagnostic when referenced in same namespace (blockless with un-blockless)", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + namespace N; + model M extends B {} + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model A {}; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + namespace N { + model B extends A {} + } + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("check with old program", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./b.tsp"; + import "./blue.js"; + namespace N; + model M extends B {} + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + namespace N; + model A {}; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + namespace N { + @blue() + model B {} + } + `, + ); + + const oldProgram = await host.compileWithProgram("main.tsp"); + expectDiagnostics(oldProgram.diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./a.tsp"`, + severity: "hint", + }, + ]); + const p = await host.compileWithProgram("main.tsp", undefined, oldProgram); + expectDiagnostics(p.diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./a.tsp"`, + severity: "hint", + }, + ]); + }); + + it("import in circle won't cause problem. Main -> a.tsp <-> b.tsp -> b2.tsp -> b.tsp, Main -> c.tsp -> d.tsp -> e.tsp -> c.tsp & b.tsp", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + import "./c.tsp"; + model BEx {...B2, ...B2Alias} + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + import "./b.tsp"; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./b2.tsp"; + import "./a.tsp"; + model B {} + `, + ); + host.addTypeSpecFile( + "b2.tsp", + ` + import "./b.tsp"; + model B2 {} + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + import "./d.tsp"; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + import "./e.tsp"; + `, + ); + host.addTypeSpecFile( + "e.tsp", + ` + import "./b.tsp"; + import "./c.tsp"; + + alias B2Alias = B2; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("import in circle won't cause problem. Main -> a.tsp <-> b.tsp -> c.tsp -> d.tsp, b.tsp <-> e.tsp <-> d.tsp, Main ref d's model", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + import "./a.tsp"; + model M { ...D } + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + import "./b.tsp"; + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./a.tsp"; + import "./c.tsp"; + import "./e.tsp"; + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + import "./d.tsp"; + `, + ); + host.addTypeSpecFile( + "d.tsp", + ` + import "./e.tsp"; + model D {} + `, + ); + host.addTypeSpecFile( + "e.tsp", + ` + import "./b.tsp"; + import "./d.tsp"; + `, + ); + + const diagnostics = await host.diagnose("main.tsp"); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + ]); + }); + + it("import from cli won't be counted as not needed", async () => { + host.addJsFile("blue.js", { $blue() {} }); + host.addTypeSpecFile( + "main.tsp", + ` + model Main {} + `, + ); + host.addTypeSpecFile( + "a.tsp", + ` + import "./b.tsp"; + model A extends B {} + `, + ); + host.addTypeSpecFile( + "b.tsp", + ` + import "./c.tsp"; + model B {} + `, + ); + host.addTypeSpecFile( + "c.tsp", + ` + model C {} + `, + ); + + const [_, diagnostics] = await host.compileAndDiagnose("main.tsp", { + additionalImports: ["./a.tsp"], + }); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./c.tsp"`, + severity: "hint", + }, + ]); + }); +}); 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 0000000000..4677becd68 --- /dev/null +++ b/packages/compiler/test/checker/unused-using.test.ts @@ -0,0 +1,878 @@ +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({ checkUnnecessaryDiagnostics: true }); + }); + + 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); + }); +}); diff --git a/packages/compiler/test/checker/using.test.ts b/packages/compiler/test/checker/using.test.ts index 736e4b5052..3560758111 100644 --- a/packages/compiler/test/checker/using.test.ts +++ b/packages/compiler/test/checker/using.test.ts @@ -1,18 +1,13 @@ 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; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("works in global scope", async () => { @@ -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,28 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnosticEmpty(diagnostics); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: 'Unused import: import "./a.tsp"', + severity: "hint", + }, + { + code: "unused-import", + message: 'Unused import: import "./b.tsp"', + severity: "hint", + }, + { + 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 +237,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 +318,28 @@ describe("compiler: using statements", () => { ); const diagnostics = await testHost.diagnose("./"); - expectDiagnosticEmpty(diagnostics); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: 'Unused import: import "./a.tsp"', + severity: "hint", + }, + { + code: "unused-import", + message: 'Unused import: import "./b.tsp"', + severity: "hint", + }, + { + 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 +373,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-import" && 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 +413,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-import" && 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 +449,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-import" && 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 +478,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-import" && 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 +531,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-import" && 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 +571,29 @@ 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-import", + message: 'Unused import: import "./a.tsp"', + severity: "hint", + }, + { + code: "unused-import", + message: 'Unused import: import "./b.tsp"', + severity: "hint", + }, + { + code: "unused-using", + message: "Unused using: using N", + severity: "hint", + }, + ]); }); it("usings are local to a file", async () => { @@ -570,7 +666,7 @@ describe("compiler: using statements", () => { describe("emit diagnostics", () => { async function diagnose(code: string) { - const testHost = await createTestHost(); + const testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addTypeSpecFile( "main.tsp", ` @@ -585,10 +681,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 +703,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", + }, + ]); }); }); }); diff --git a/packages/compiler/test/cli.test.ts b/packages/compiler/test/cli.test.ts index 872f2e7797..cee6f85f9c 100644 --- a/packages/compiler/test/cli.test.ts +++ b/packages/compiler/test/cli.test.ts @@ -18,7 +18,7 @@ describe("compiler: cli", () => { const cwd = resolveVirtualPath("ws"); beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); host.addTypeSpecFile("ws/main.tsp", ""); }); diff --git a/packages/compiler/test/core/codefixes.test.ts b/packages/compiler/test/core/codefixes.test.ts index 5d1b18d559..e4ef1c36cd 100644 --- a/packages/compiler/test/core/codefixes.test.ts +++ b/packages/compiler/test/core/codefixes.test.ts @@ -17,7 +17,7 @@ describe("Codefixes", () => { fix: (context: CodeFixContext, file: SourceFile) => CodeFixEdit | CodeFixEdit[], ): Promise { const fakeFile = createSourceFile(text, "test.ts"); - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); let result: string | undefined; await applyCodeFix( { diff --git a/packages/compiler/test/core/emitter-options.test.ts b/packages/compiler/test/core/emitter-options.test.ts index cf479578d3..014db9ea37 100644 --- a/packages/compiler/test/core/emitter-options.test.ts +++ b/packages/compiler/test/core/emitter-options.test.ts @@ -24,7 +24,7 @@ describe("compiler: emitter options", () => { options: Record, ): Promise<[EmitContext | undefined, readonly Diagnostic[]]> { let emitContext: EmitContext | undefined; - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); host.addTypeSpecFile("main.tsp", ""); host.addTypeSpecFile( "node_modules/fake-emitter/package.json", diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts index a7d467752b..2e880e610b 100644 --- a/packages/compiler/test/core/linter.test.ts +++ b/packages/compiler/test/core/linter.test.ts @@ -34,7 +34,7 @@ describe("compiler: linter", () => { code: string | Record, linterDef: LinterDefinition, ): Promise { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); if (typeof code === "string") { host.addTypeSpecFile("main.tsp", code); } else { @@ -151,7 +151,7 @@ describe("compiler: linter", () => { const files = { "main.tsp": ` import "my-lib"; - model Bar {} + model Bar extends Foo {} `, "node_modules/my-lib/package.json": JSON.stringify({ name: "my-lib", tspMain: "main.tsp" }), "node_modules/my-lib/main.tsp": "model Foo {}", @@ -166,7 +166,7 @@ describe("compiler: linter", () => { const files = { "main.tsp": ` import "my-lib"; - model Foo {} + model Foo extends Bar {} `, "node_modules/my-lib/package.json": JSON.stringify({ name: "my-lib", tspMain: "main.tsp" }), "node_modules/my-lib/main.tsp": "model Bar {}", @@ -174,11 +174,13 @@ describe("compiler: linter", () => { const linter = await createTestLinterAndEnableRules(files, { rules: [noModelFoo], }); - expectDiagnostics(linter.lint(), { - severity: "warning", - code: "@typespec/test-linter/no-model-foo", - message: `Cannot call model 'Foo'`, - }); + expectDiagnostics(linter.lint(), [ + { + severity: "warning", + code: "@typespec/test-linter/no-model-foo", + message: `Cannot call model 'Foo'`, + }, + ]); }); }); @@ -252,7 +254,7 @@ describe("compiler: linter", () => { describe("(integration) loading in program", () => { async function diagnoseReal(code: string) { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); host.addTypeSpecFile("main.tsp", code); host.addTypeSpecFile( "node_modules/my-lib/package.json", diff --git a/packages/compiler/test/decorator-utils.test.ts b/packages/compiler/test/decorator-utils.test.ts index dfb70ccc8d..6103fd2970 100644 --- a/packages/compiler/test/decorator-utils.test.ts +++ b/packages/compiler/test/decorator-utils.test.ts @@ -19,7 +19,7 @@ import { describe("compiler: decorator utils", () => { describe("typespecTypeToJson", () => { async function convertDecoratorDataToJson(code: string) { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); let result: any; // add test decorators @@ -141,7 +141,7 @@ describe("compiler: decorator utils", () => { describe("validateDecoratorUniqueOnNode", () => { let runner: BasicTestRunner; beforeEach(async () => { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host, { wrapper: (x) => `import "./lib.js";\n${x}` }); function $bar(context: DecoratorContext, target: Type) { @@ -208,7 +208,7 @@ describe("compiler: decorator utils", () => { let runner: BasicTestRunner; beforeEach(async () => { - const host = await createTestHost(); + const host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host, { wrapper: (x) => `import "./lib.js";\n${x}` }); function $red(context: DecoratorContext, target: Type) { diff --git a/packages/compiler/test/decorators/tags.test.ts b/packages/compiler/test/decorators/tags.test.ts index bce4a17f8f..cb2a2b9be4 100644 --- a/packages/compiler/test/decorators/tags.test.ts +++ b/packages/compiler/test/decorators/tags.test.ts @@ -8,7 +8,7 @@ describe("compiler: tag decorator", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("applies @tag decorator to namespaces, interfaces, and operations", async (): Promise => { diff --git a/packages/compiler/test/e2e/scenarios/scenarios.e2e.ts b/packages/compiler/test/e2e/scenarios/scenarios.e2e.ts index fb10c2d393..0d69bf8595 100644 --- a/packages/compiler/test/e2e/scenarios/scenarios.e2e.ts +++ b/packages/compiler/test/e2e/scenarios/scenarios.e2e.ts @@ -21,7 +21,11 @@ describe("compiler: entrypoints", () => { it("compile library with TypeSpec entrypoint", async () => { const program = await compileScenario("typespec-lib"); - expectDiagnosticEmpty(program.diagnostics); + expectDiagnostics(program.diagnostics, { + code: "unused-import", + message: `Unused import: import "./lib.js"`, + severity: "hint", + }); }); it("emit diagnostics if library has invalid main", async () => { @@ -36,7 +40,11 @@ describe("compiler: entrypoints", () => { const program = await compileScenario("emitter-with-typespec", { emit: ["@typespec/test-emitter-with-typespec"], }); - expectDiagnosticEmpty(program.diagnostics); + expectDiagnostics(program.diagnostics, { + code: "unused-import", + message: `Unused import: import "./lib.js"`, + severity: "hint", + }); }); }); @@ -134,21 +142,32 @@ describe("compiler: entrypoints", () => { const program = await compileScenario("same-library-same-version", { emit: ["@typespec/lib2"], }); - expectDiagnosticEmpty(program.diagnostics); + expectDiagnostics(program.diagnostics, { + code: "unused-import", + message: `Unused import: import "@typespec/lib1"`, + severity: "hint", + }); }); it("emit error if loading different install of the same library at different version", async () => { const program = await compileScenario("same-library-diff-version", { emit: ["@typespec/lib2"], }); - expectDiagnostics(program.diagnostics, { - code: "incompatible-library", - message: [ - `Multiple versions of "@typespec/my-lib" library were loaded:`, - ` - Version: "1.0.0" installed at "${scenarioRoot}/same-library-diff-version/node_modules/@typespec/lib1"`, - ` - Version: "2.0.0" installed at "${scenarioRoot}/same-library-diff-version/node_modules/@typespec/lib2"`, - ].join("\n"), - }); + expectDiagnostics(program.diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "@typespec/lib1"`, + severity: "hint", + }, + { + code: "incompatible-library", + message: [ + `Multiple versions of "@typespec/my-lib" library were loaded:`, + ` - Version: "1.0.0" installed at "${scenarioRoot}/same-library-diff-version/node_modules/@typespec/lib1"`, + ` - Version: "2.0.0" installed at "${scenarioRoot}/same-library-diff-version/node_modules/@typespec/lib2"`, + ].join("\n"), + }, + ]); }); it("Back compat: succeed if main.cadl exists", async () => { diff --git a/packages/compiler/test/experimental/mutator.test.ts b/packages/compiler/test/experimental/mutator.test.ts index 7ccac8bcfd..ec8a2d87d8 100644 --- a/packages/compiler/test/experimental/mutator.test.ts +++ b/packages/compiler/test/experimental/mutator.test.ts @@ -16,7 +16,7 @@ let host: TestHost; let runner: BasicTestRunner; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = createTestWrapper(host); }); diff --git a/packages/compiler/test/init/init-template.test.ts b/packages/compiler/test/init/init-template.test.ts index 212dce0bf3..87f9a340d3 100644 --- a/packages/compiler/test/init/init-template.test.ts +++ b/packages/compiler/test/init/init-template.test.ts @@ -11,7 +11,7 @@ import { TestHost, createTestHost, resolveVirtualPath } from "../../src/testing/ describe("compiler: init: templates", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function getOutputFile(path: string): string | undefined { diff --git a/packages/compiler/test/libraries/libraries.test.ts b/packages/compiler/test/libraries/libraries.test.ts index b1d9c4d0c2..68bf8facb0 100644 --- a/packages/compiler/test/libraries/libraries.test.ts +++ b/packages/compiler/test/libraries/libraries.test.ts @@ -28,7 +28,7 @@ describe("compiler: libraries", () => { } it("detects compiler version mismatches", async () => { - const testHost = await createTestHost(); + const testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addTypeSpecFile("main.tsp", ""); testHost.addTypeSpecFile( "./node_modules/@typespec/compiler/package.json", @@ -48,7 +48,7 @@ describe("compiler: libraries", () => { }); it("allows compiler install to mismatch if the version are the same", async () => { - const testHost = await createTestHost(); + const testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addTypeSpecFile("main.tsp", ""); testHost.addTypeSpecFile( "./node_modules/@typespec/compiler/package.json", @@ -60,7 +60,7 @@ describe("compiler: libraries", () => { }); it("report errors in js files", async () => { - const testHost = await createTestHost(); + const testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); testHost.addJsFile("lib1.js", { $myDec: () => null }); testHost.addJsFile("lib2.js", { $myDec: () => null }); testHost.addTypeSpecFile( diff --git a/packages/compiler/test/name-resolver.test.ts b/packages/compiler/test/name-resolver.test.ts index e336595fb0..5a5defa5f6 100644 --- a/packages/compiler/test/name-resolver.test.ts +++ b/packages/compiler/test/name-resolver.test.ts @@ -7,6 +7,7 @@ import { createLogger } from "../src/core/logger/logger.js"; import { createTracer } from "../src/core/logger/tracer.js"; import { createResolver, NameResolver } from "../src/core/name-resolver.js"; import { getNodeAtPosition, parse } from "../src/core/parser.js"; +import { SourceResolution } from "../src/core/source-loader.js"; import { IdentifierNode, JsSourceFileNode, @@ -29,7 +30,15 @@ let program: Program; beforeEach(() => { program = createProgramShim(); binder = createBinder(program); - resolver = createResolver(program); + const mockSourceResolution: SourceResolution = { + sourceFiles: new Map(), + jsSourceFiles: new Map(), + sourceFileImportedBy: new Map(), + locationContexts: new WeakMap(), + loadedLibraries: new Map(), + diagnostics: [], + }; + resolver = createResolver(program, mockSourceResolution); }); describe("model statements", () => { diff --git a/packages/compiler/test/projection/projection-logic.test.ts b/packages/compiler/test/projection/projection-logic.test.ts index bd8f05a6e7..99333daeaf 100644 --- a/packages/compiler/test/projection/projection-logic.test.ts +++ b/packages/compiler/test/projection/projection-logic.test.ts @@ -25,7 +25,7 @@ describe("compiler: projections: logic", () => { let testHost: TestHost; beforeEach(async () => { - testHost = await createTestHost(); + testHost = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); it("projects nested namespaces", async () => { diff --git a/packages/compiler/test/projection/projector-identity.test.ts b/packages/compiler/test/projection/projector-identity.test.ts index 5a0bfdb85c..15e6dbf6cf 100644 --- a/packages/compiler/test/projection/projector-identity.test.ts +++ b/packages/compiler/test/projection/projector-identity.test.ts @@ -2,6 +2,7 @@ import { deepStrictEqual, ok, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { DecoratorContext, Namespace, Type, getTypeName, isType } from "../../src/core/index.js"; import { createProjector } from "../../src/core/projector.js"; +import { expectDiagnostics } from "../../src/testing/expect.js"; import { createTestHost, createTestRunner } from "../../src/testing/test-host.js"; import { BasicTestRunner, TestHost } from "../../src/testing/types.js"; @@ -13,7 +14,7 @@ describe("compiler: projector: Identity", () => { let runner: BasicTestRunner; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); runner = await createTestRunner(host); }); @@ -43,9 +44,25 @@ describe("compiler: projector: Identity", () => { $track: (_: DecoratorContext, target: Type) => trackedTypes.push(target), }); - const { target } = await runner.compile(` + const mergedCode = ` import "./track.js"; - ${code}`); + ${code}`; + + let target: Type; + if (code.includes("@track")) { + const r = await runner.compile(mergedCode); + target = r.target; + } else { + const [r, diagnostics] = await runner.compileAndDiagnose(mergedCode); + expectDiagnostics(diagnostics, [ + { + code: "unused-import", + message: `Unused import: import "./track.js"`, + severity: "hint", + }, + ]); + target = r.target; + } while (trackedTypes.length > 0) { trackedTypes.pop(); diff --git a/packages/compiler/test/semantic-walker.test.ts b/packages/compiler/test/semantic-walker.test.ts index 8d35b1c883..b402ebbbfb 100644 --- a/packages/compiler/test/semantic-walker.test.ts +++ b/packages/compiler/test/semantic-walker.test.ts @@ -25,7 +25,7 @@ describe("compiler: semantic walker", () => { let host: TestHost; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); function createCollector(customListener?: SemanticNodeListener) { diff --git a/packages/compiler/test/suppression.test.ts b/packages/compiler/test/suppression.test.ts index 236dfbf47d..064798bcfd 100644 --- a/packages/compiler/test/suppression.test.ts +++ b/packages/compiler/test/suppression.test.ts @@ -11,7 +11,7 @@ describe("compiler: suppress", () => { let host: TestHost; beforeEach(async () => { - host = await createTestHost(); + host = await createTestHost({ checkUnnecessaryDiagnostics: true }); }); async function run(typespec: string) { diff --git a/packages/http-specs/specs/server/path/multiple/main.tsp b/packages/http-specs/specs/server/path/multiple/main.tsp index 868684aeb3..3535d7c585 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 3f6b6d8979..83a75e0bf2 100644 --- a/packages/http-specs/specs/special-headers/conditional-request/main.tsp +++ b/packages/http-specs/specs/special-headers/conditional-request/main.tsp @@ -1,10 +1,8 @@ import "@typespec/http"; -import "@typespec/versioning"; 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 e9bb0067ef..0ab5192eec 100644 --- a/packages/http-specs/specs/special-headers/repeatability/main.tsp +++ b/packages/http-specs/specs/special-headers/repeatability/main.tsp @@ -1,10 +1,8 @@ import "@typespec/http"; -import "@typespec/versioning"; 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/responses.test.ts b/packages/http/test/responses.test.ts index 99a23376ea..b22619a220 100644 --- a/packages/http/test/responses.test.ts +++ b/packages/http/test/responses.test.ts @@ -161,7 +161,9 @@ it("empty response model becomes body if it has children", async () => { `, ); - expectDiagnosticEmpty(diagnostics); + expectDiagnosticEmpty( + diagnostics.filter((d) => d.code !== "unused-import" && d.code !== "unused-using"), + ); strictEqual(routes.length, 1); const responses = routes[0].responses; strictEqual(responses.length, 1); diff --git a/packages/playground-website/samples/unions.tsp b/packages/playground-website/samples/unions.tsp index 6a6746d1af..fc88e107fe 100644 --- a/packages/playground-website/samples/unions.tsp +++ b/packages/playground-website/samples/unions.tsp @@ -1,12 +1,10 @@ import "@typespec/http"; -import "@typespec/rest"; import "@typespec/openapi3"; @service({ 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 564e048506..8b62df4d93 100644 --- a/packages/rest/test/test-host.ts +++ b/packages/rest/test/test-host.ts @@ -87,15 +87,23 @@ export async function getOperationsWithServiceNamespace( routeOptions?: RouteResolutionOptions, ): Promise<[HttpOperation[], readonly Diagnostic[]]> { const runner = await createRestTestRunner(); - await runner.compileAndDiagnose( + const [_, diagnostics] = await runner.compileAndDiagnose( `@service({title: "Test Service"}) namespace TestService; ${code}`, { noEmit: true, }, ); + const originalDiagnosticsLength = runner.program.diagnostics.length; const [services] = getAllHttpServices(runner.program, routeOptions); - return [services[0].operations, runner.program.diagnostics]; + // the diagnostics from compileAndDiagnose may be different from program.diagnostic according to some test configuration + // (i.e. having the unnecessary diagnostics filtered out), so here return the combination of diagnostics from compileAndDiagnose + // and getAllHttpServices + const allDiagnostics = [ + ...diagnostics, + ...runner.program.diagnostics.slice(originalDiagnosticsLength), + ]; + return [services[0].operations, allDiagnostics]; } export async function getOperations(code: string): Promise { diff --git a/packages/samples/src/sample-snapshot-testing.ts b/packages/samples/src/sample-snapshot-testing.ts index a7f34bd433..dc00d67017 100644 --- a/packages/samples/src/sample-snapshot-testing.ts +++ b/packages/samples/src/sample-snapshot-testing.ts @@ -106,7 +106,9 @@ function defineSampleSnaphotTest( } const program = await compile(host, sample.fullPath, options); - expectDiagnosticEmpty(program.diagnostics); + expectDiagnosticEmpty( + program.diagnostics.filter((d) => d.code !== "unused-import" && d.code !== "unused-using"), + ); if (shouldUpdateSnapshots) { try {