π Unused imports when the import is used inside JSDoc syntax
Environment information
CLI:
Version: 1.9.4
Color support: true
Platform:
CPU Architecture: x86_64
OS: linux
Environment:
BIOME_LOG_PATH: unset
BIOME_LOG_PREFIX_NAME: unset
BIOME_CONFIG_PATH: unset
NO_COLOR: unset
TERM: "xterm-256color"
JS_RUNTIME_VERSION: unset
JS_RUNTIME_NAME: unset
NODE_PACKAGE_MANAGER: unset
Biome Configuration:
Status: Loaded successfully
Formatter disabled: false
Linter disabled: false
Organize imports disabled: true
VCS disabled: true
Linter:
JavaScript enabled: true
JSON enabled: true
CSS enabled: true
GraphQL enabled: false
Recommended: true
All: false
Enabled rules:
performance/noDelete
suspicious/noCatchAssign
suspicious/noUnsafeNegation
complexity/useLiteralKeys
suspicious/noClassAssign
style/useImportType
complexity/noMultipleSpacesInRegularExpressionLiterals
a11y/useValidLang
complexity/noUselessEmptyExport
suspicious/useNamespaceKeyword
suspicious/useValidTypeof
a11y/useValidAriaRole
suspicious/noAssignInExpressions
a11y/useAriaActivedescendantWithTabindex
suspicious/noDuplicateParameters
style/useDefaultParameterLast
complexity/noEmptyTypeParameters
correctness/noConstructorReturn
style/useSelfClosingElements
suspicious/noDuplicateSelectorsKeyframeBlock
correctness/noUnknownProperty
correctness/noUnusedLabels
complexity/noUselessTernary
correctness/noUnreachableSuper
suspicious/noCompareNegZero
suspicious/noExplicitAny
correctness/noSwitchDeclarations
a11y/noAutofocus
correctness/noUnsafeOptionalChaining
correctness/noConstAssign
suspicious/noControlCharactersInRegex
complexity/noUselessTypeConstraint
style/noVar
suspicious/noDoubleEquals
suspicious/noRedundantUseStrict
style/useLiteralEnumMembers
suspicious/noGlobalIsNan
suspicious/noEmptyInterface
suspicious/noConstEnum
suspicious/noMisleadingCharacterClass
correctness/noPrecisionLoss
a11y/noLabelWithoutControl
suspicious/noRedeclare
correctness/noStringCaseMismatch
correctness/noSetterReturn
correctness/noInvalidConstructorSuper
suspicious/noImplicitAnyLet
suspicious/noFallthroughSwitchClause
suspicious/noUnsafeDeclarationMerging
complexity/noUselessThisAlias
correctness/noUnreachable
a11y/useKeyWithClickEvents
suspicious/noDuplicateObjectKeys
complexity/noThisInStatic
complexity/useOptionalChain
correctness/noInnerDeclarations
suspicious/noDuplicateCase
a11y/useValidAnchor
complexity/useRegexLiterals
correctness/noSelfAssign
correctness/noInvalidBuiltinInstantiation
style/useShorthandFunctionType
suspicious/noShadowRestrictedNames
correctness/noInvalidDirectionInLinearGradient
suspicious/noImportantInKeyframe
a11y/useMediaCaption
complexity/noUselessLabel
complexity/noUselessCatch
correctness/noUnsafeFinally
a11y/useAriaPropsForRole
correctness/noNonoctalDecimalEscape
style/useEnumInitializers
a11y/useHtmlLang
suspicious/noDuplicateTestHooks
complexity/noStaticOnlyClass
style/useWhile
complexity/useArrowFunction
style/noInferrableTypes
a11y/noNoninteractiveTabindex
complexity/useSimpleNumberKeys
correctness/useYield
a11y/noInteractiveElementToNoninteractiveRole
style/useNumericLiterals
correctness/noUnnecessaryContinue
suspicious/noApproximativeNumericConstant
suspicious/noImportAssign
suspicious/noLabelVar
correctness/noGlobalObjectCalls
suspicious/useDefaultSwitchClauseLast
correctness/noUnknownUnit
a11y/useAltText
correctness/noEmptyCharacterClassInRegex
suspicious/noSparseArray
a11y/useIframeTitle
complexity/noBannedTypes
a11y/noSvgWithoutTitle
correctness/noVoidElementsWithChildren
style/useAsConstAssertion
correctness/useJsxKeyInIterable
style/useExportType
suspicious/noDebugger
complexity/noUselessLoneBlockStatements
style/noArguments
a11y/useValidAriaValues
suspicious/noCommentText
a11y/useFocusableInteractive
correctness/noUnmatchableAnbSelector
suspicious/noMisleadingInstantiator
suspicious/noDuplicateJsxProps
suspicious/noGlobalAssign
a11y/noPositiveTabindex
correctness/noEmptyPattern
complexity/noExcessiveNestedTestSuites
security/noDangerouslySetInnerHtmlWithChildren
a11y/useKeyWithMouseEvents
suspicious/noExtraNonNullAssertion
suspicious/noPrototypeBuiltins
correctness/noRenderReturnValue
correctness/useExhaustiveDependencies
security/noGlobalEval
style/noNonNullAssertion
a11y/noRedundantRoles
complexity/useFlatMap
correctness/useIsNan
style/useConst
suspicious/noGlobalIsFinite
suspicious/noSelfCompare
suspicious/noShorthandPropertyOverrides
suspicious/noAsyncPromiseExecutor
suspicious/noDuplicateFontNames
suspicious/noThenProperty
suspicious/useGetterReturn
security/noDangerouslySetInnerHtml
style/useNodejsImportProtocol
a11y/noDistractingElements
suspicious/noArrayIndexKey
complexity/noWith
suspicious/noDuplicateClassMembers
complexity/noExtraBooleanCast
performance/noAccumulatingSpread
a11y/useValidAriaProps
a11y/noRedundantAlt
correctness/noChildrenProp
correctness/noUnknownFunction
correctness/noInvalidPositionAtImportRule
suspicious/noConfusingLabels
suspicious/noSuspiciousSemicolonInJsx
suspicious/noConfusingVoidType
suspicious/noFocusedTests
a11y/useButtonType
a11y/useSemanticElements
a11y/noAriaUnsupportedElements
correctness/noInvalidGridAreas
correctness/noFlatMapIdentity
a11y/noBlankTarget
a11y/useHeadingContent
correctness/useValidForDirection
correctness/noVoidTypeReturn
correctness/noInvalidUseBeforeDeclaration
a11y/noAriaHiddenOnFocusable
a11y/useGenericFontNames
correctness/noUnknownMediaFeatureName
a11y/useAnchorContent
complexity/noUselessRename
style/useNumberNamespace
complexity/noUselessConstructor
a11y/noAccessKey
style/useExponentiationOperator
style/noUnusedTemplateLiteral
complexity/noUselessSwitchCase
style/useSingleVarDeclarator
suspicious/noExportsInTest
a11y/noNoninteractiveElementToInteractiveRole
style/noCommaOperator
suspicious/noDuplicateAtImportRules
suspicious/useIsArray
a11y/noHeaderScope
complexity/noUselessFragments
suspicious/noMisrefactoredShorthandAssign
suspicious/noEmptyBlock
complexity/noForEach
correctness/noUnusedImports
suspicious/noFunctionAssign
Workspace:
Open Documents: 0
Rule name
lint/correctness/noUnusedImports
Playground link
https://biomejs.dev/playground/?indentStyle=space&lintRules=all&files.main.js=aQBtAHAAbwByAHQAIAB7AFMAbwBtAGUAQwBsAGEAcwBzAH0AIABmAHIAbwBtACAAIgAuAC8AbwB0AGgAZQByAGYAaQBsAGUALgBqAHMAIgA7AAoACgAvACoAKgAKACAAKgAgAEAAcABhAHIAYQBtACAAewBTAG8AbQBlAEMAbABhAHMAcwB9ACAAYQAKACAAKgAvAAoAZQB4AHAAbwByAHQAIABmAHUAbgBjAHQAaQBvAG4AIAB4ACgAYQApACAAewAKACAAIAByAGUAdAB1AHIAbgAgAGEALgBiACAAKwAgADUAOwAKAH0A
Expected result
As the import is referenced in the JSDoc, it should not be flagged as an unused import.
Removing this import causes tsc to complain (rightfully) that it doesn't know what type this is. (tsc checking JS files). This happens a lot when you are passing a certain type to a function, but never need to create it. You never explicitly reference the typename but access its properties.
Code of Conduct
- [X] I agree to follow Biome's Code of Conduct
Thanks, this seems like a valid request.
Please be aware though that we currently donβt do any analysis on JsDoc comments, so this is unlikely to be picked up soon unless someone wishes to contribute such support.
As a workaround, I would suggest moving the import itself into a comment as well. I believe tsc supports some syntax to do this.
Just stumbled over this issue after I migrated to Biome these days. Another relevant JSDoc syntax accessing types is: {@link TypeName}, {@link TypeName.member}, {@link TypeName#member} (https://tsdoc.org/pages/tags/link/ and https://jsdoc.app/tags-inline-link)
It seems with https://github.com/biomejs/biome/pull/5599 Biome made the required step to do JSDoc type resolving. Is there anything more needed for this issue here, or does #5599 cover it already?
@Danielku15 What #5599 achieved is that we can now look up JSDoc comments of imported symbols. I don't think that's really what this issue is about, since it's about marking imported symbols as used when a local JSDoc comment makes a reference to them.
To implement the necessary support raised in this issue, noUnusedImports should be extended to scan the module for JSDoc comments, and interpret them, keeping in mind the syntaxes you mentioned, to see if there are any references to the imported symbols. Neither of these things are really helped by #5599 unfortunately.
Thanks for the insights. I was searching the repository related to existing bits on JSDoc parsing and handling and the new PR looked promising π
I'm very new to Rust so contributing would be tough without some foundation in-place π From a gut-feeling:
- The AST would need extension to hold the related comments (maybe moving the new JsdocComment and related bits to biome_js_syntax makes sense).
- Parse the JsDoc syntax into dedicated AST nodes according to spec. To minimize the performance impact on existing projects it could be a new option to enable JSDoc/TSDoc parsing. This should likely happen in biome_js_parser
- On the semantic model (somehow) the type references need to be bound and registered.
- The
no_unused_importsrule would pick up the newly registered references from the semantic model.
Are my assumptions correct? Do you have any hints which bits of the code I could explore deeper to learn about the required bits?
@Danielku15 I don't think it's as much work as that, to be honest.
Our CST (it's not an AST) already has the comments, so no extensions are necessary there. JsdocComment::try_from(node: &JsSyntaxNode) can be used for extracting JSDoc comments from any existing syntax nodes, it just means that noUnusedImports needs to traverse almost every node, because JSDoc comments may exist almost anywhere in the document.
I also don't think I would write a full blown parser for JSDoc comments either, since the format is pretty relaxed to begin with. Just scan the comment lines for the tag you're looking for, and if found, interpret the following part per spec.
As for the semantic model, I think I wouldn't involve it here. Just let the rule create a hash map with symbols discovered in JSDoc type references, and before letting noUnusedImports signal a diagnostic, let it check the map to see if the symbol is in there. We can always try to generalise things if more rules want access to this kind of information, but I wouldn't try to complicate it from the start.
And this way, we probably can keep the performance sufficiently under control that we don't need to create additional options for it. Or if we do need it, it can simply be kept on the rule itself.
Time to learn some Rust π Lets see how far I get
@arendjr
As for the semantic model, I think I wouldn't involve it here. Just let the rule create a hash map with symbols discovered in JSDoc type references, and before letting noUnusedImports signal a diagnostic, let it check the map to see if the symbol is in there. We can always try to generalise things if more rules want access to this kind of information, but I wouldn't try to complicate it from the start.
I got a bit stuck on where I could collect and store the types as part of the NoUnusedImports rule itself. With ctx.root() I could get the AST (a copy at least) and traverse it. But as run is called for every matched node I need some mechanism to collect and store the list of types once per file before the actual import nodes are analyzed and diagnostics are created.
Is there a good way to traverse the CST once per file and remember some stuff for usage in run and diagnostics?
I also don't think I would write a full blown parser for JSDoc comments either, since the format is pretty relaxed to begin with. Just scan the comment lines for the tag you're looking for, and if found, interpret the following part per spec.
I created a bunch of test files to begin with. The syntax can get quite complex for JSDoc https://jsdoc.app/tags-type but maybe some existing parser for TypeScript type nodes can help there in future.
issue_4677_jsdoc_syntax.js
// See https://github.com/biomejs/biome/issues/4677
// https://jsdoc.app/tags-type
import SymbolName from "mod";
import MultipleTypes from "mod";
import ArraysGeneric from "mod";
import ArraysBrackets from "mod";
import AnyGenerics from "mod";
import ObjectProperties from "mod";
import Nullable from "mod";
import NonNullable from "mod";
import VarArgs from "mod";
import OptionalParameter from "mod";
/**
* @type {SymbolName}
*/
let testSymbolName;
/**
* @type {(number|MultipleTypes)}
*/
let testMultipleTypes;
/**
* @type {Array.<ArraysGeneric>}
*/
let testArraysGeneric;
/**
* @type {ArraysBrackets[]}
*/
let testArraysBrackets;
/**
* @type {Object.<string, AnyGenerics>}
*/
let testAnyGenerics;
/**
* @type {{a:number, b:ObjectProperties}}
*/
let testObjectProperties;
/**
* @type {?Nullable}
*/
let testNullable;
/**
* @type {!NonNullable}
*/
let testNonNullable;
/**
* @param {...VarArgs} params
*/
function testVarArgs(params) {}
/**
* @param {OptionalParameter=} optional
*/
function testOptionalParameter(optional) {}
issue_4677_jsdoc.js
// See https://github.com/biomejs/biome/issues/4677
// some types for the different locations
import TypeOnFunctionParam from "mod";
import TypeOnClassMethodParam from "mod";
import TypeOnClassConstructorParam from "mod";
import TypeOnClassField from "mod";
import TypeOnGlobalVariable from "mod";
import TypeOnFunctionVariable from "mod";
import TypeOnTypeDef from "mod";
/**
* @typedef {TypeOnTypeDef} TestTypeOnTypeDef
*/
/**
* @param param {TypeOnFunctionParam}
*/
function testTypeOnFunction(param) {}
class TestTypeOnClassMethodParam {
/**
* @param {TypeOnClassMethodParam} param
*/
method(param) {}
}
class TestTypeOnClassConstructorParam {
/**
* @param {TypeOnClassConstructorParam} param
*/
constructor(param) {}
}
class TestTypeOnClassField {
constructor(param) {
/**
* @type {TypeOnClassField}
*/
this.var = 10;
}
}
/**
* @type {TypeOnGlobalVariable}
*/
let testTypeOnGlobalVariable;
function testTypeOnFunctionVariable() {
/**
* @type {TypeOnFunctionVariable}
*/
let testTypeOnFunctionVariable;
}
issue_4677_tsdoc_syntax.ts
// See https://github.com/biomejs/biome/issues/4677
import type SyntaxNormal from "mod";
import type SyntaxWithDotMember from "mod";
import type SyntaxWithHashMember from "mod";
import type SyntaxWithDotMemberAndPipeDescription from "mod";
import type SyntaxWithHashMemberAndPipeDescription from "mod";
import type SyntaxWithDotMemberAnSpaceDescription from "mod";
import type SyntaxWithHashMemberAndSpaceDescription from "mod";
/**
* {@link SyntaxNormal}
*/
function testSyntaxNormal() { }
/**
* {@link SyntaxWithDotMember.member}
*/
function testSyntaxWithDotMember() { }
/**
* {@link SyntaxWithHashMember#member}
*/
function testSyntaxWithHashMember() { }
/**
* {@link SyntaxWithDotMemberAndPipeDescription.member|Description}
*/
function testSyntaxWithDotMemberAndPipeDescription() { }
/**
* {@link SyntaxWithHashMemberAndPipeDescription#member|Description}
*/
function testSyntaxWithHashMemberAndPipeDescription() { }
/**
* {@link SyntaxWithDotMemberAnSpaceDescription.member Description}
*/
function testSyntaxWithDotMemberAnSpaceDescription() { }
/**
* {@link SyntaxWithHashMemberAndSpaceDescription#member Description}
*/
function testSyntaxWithHashMemberAndSpaceDescription() { }
issue_4677_tsdoc.ts
// See https://github.com/biomejs/biome/issues/4677
// some types for the different locations
import type LinkOnFunction from "mod";
import type LinkOnVariable from "mod";
import type LinkOnClass from "mod";
import type LinkOnClassField from "mod";
import type LinkOnClassMethod from "mod";
import type LinkOnClassConstructor from "mod";
import type LinkOnClassAccessor from "mod";
import type LinkOnClassIndexer from "mod";
import type LinkOnInterface from "mod";
import type LinkOnInterfaceField from "mod";
import type LinkOnInterfaceMethod from "mod";
import type LinkOnInterfaceIndexer from "mod";
/**
* {@link LinkOnFunction}
*/
function testLinkOnFunction() { }
/**
* {@link LinkOnVariable}
*/
const testLinkOnVariable = 3;
/**
* {@link LinkOnClass}
*/
class TestLinkOnClass { }
class TestLinkOnClassField {
/**
* {@link LinkOnClassField}
*/
field: number;
}
class TestLinkOnClassMethod {
/**
* {@link LinkOnClassMethod}
*/
method(): void { }
}
class TestLinkOnClassConstructor {
/**
* {@link LinkOnClassConstructor}
*/
constructor() { }
}
class TestLinkOnClassAccessor {
/**
* {@link LinkOnClassAccessor}
*/
get accessor(): number { return 0 }
}
class TestLinkOnClassIndexer {
/**
* {@link LinkOnClassIndexer}
*/
[index: number]: string;
}
/**
* {@link LinkOnInterface}
*/
interface TestLinkOnInterface { }
interface TestLinkOnInterfaceField {
/**
* {@link LinkOnInterfaceField}
*/
field: string;
}
interface TestLinkOnInterfaceMethod {
/**
* {@link LinkOnInterfaceMethod}
*/
method(): string;
}
interface TestLinkOnInterfaceIndexer {
/**
* {@link LinkOnInterfaceIndexer}
*/
[index: number]: string;
}
Is there a good way to traverse the CST once per file and remember some stuff for usage in run and diagnostics?
Oh yes, sounds like that might indeed be what you need here. It's essentially what the semantic model itself does as well, but instead of using that model you'll just create your own small model and use that instead. For another example of how to do that, see the EarlyReturnDetectionVisitor in use_hook_at_top_level.rs.
And yes, you're right the type annotations can get somewhat complex indeed, maybe those require a tiny parser indeed. I was more referring to extracting the tags themselves. That part you can do pretty ad-hoc, I think, and then extract the type value, and restrict your type parser to only interpreting the right values.
A first proposal is now available via https://github.com/biomejs/biome/pull/5698. Bear with me on the style and proper syntax bits of Rust. My very first piece of work in Rust π₯³
TBH it was mostly a matter of adapting what I've learned from looking how things are done left and right. Still learning π
Since https://github.com/biomejs/biome/pull/5698 was merged, I think the issue should be closed now?
I think no, since the issue still reproduces on the playground in the description
Ok this is interesting because the PR included snapshot tests that would cover the issue reproduced in playground. I can take a look at this issue to see if there's remaining work to do.
The unhandled remaining case is exported clauses.
In the visitor implementation, I tried printing the variables out and got
event: Enter([email protected])
node: [email protected]
token: [email protected] "function" [] [Whitespace(" ")]
for an exported function.
The same function, if not exported, will result to
event: Enter([email protected])
node: [email protected]
token: [email protected] "function" [Newline("\n"), Newline("\n"), Comments("/**\n * @param {TypeOn ..."), Newline("\n")] [Whitespace(" ")]
I think the issue might be outside of the visitor implementation. I'm gonna have to dive deeper.
I think I managed to make a fix in https://github.com/biomejs/biome/pull/6565.
Fixed by #5698 and #6565.