Fix todo-writer misses for override vals, annotated defs, and symboli… - #69
Merged
Conversation
…c names
Three classes of declarations were silently skipped by the todo-writer,
causing Scaladoc stubs to not be generated for them:
1. `override val` / `override var` declarations were excluded from
`findUndocumentedResults` (only Def/Class/Trait were checked).
2. Declarations with an annotation on the same line (e.g.
`@inline override def newArray`) were missed because a greedy regex
in `declKeywordOnLine` / `declLeadingKeyword` consumed following
keywords (like `def`) alongside the annotation, due to `\s` being
included in the annotation character class.
3. Methods with symbolic names (e.g. `<:<`, `::`, `+=`) were skipped
because `parseDef` only accepted identifier-style name characters
(letters, digits, `_`, `$`) and produced empty names for operator
names.
Fix: extend the kind check to include Val/Var, replace the greedy
annotation-stripping regex with the existing `dropLeadingAnnotations`
helper in both methods, and add a symbolic-name parsing branch to
`parseDef` using a new `isSymbolChar` predicate. Update `DeclStartPattern`
with a more precise annotation regex for consistency.
Also make `dropLeadingAnnotations` public to allow reuse across files.
Collaborator
|
@codex review |
LLM-assisted fix and regression tests, reviewed and validated locally.
LLM-assisted fix and regression tests, reviewed and validated locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In scala#26657, several declarations were noticed missing Scaladoc stubs that the todo-writer tool failed to
generate. Three root causes were identified:
Root Causes & Fixes
findUndocumentedResults in ScaladocChecker.scala only checked for DeclKind.Def, DeclKind.Class, and DeclKind.Trait ΓÇö
Val and Var were excluded. For example, override val typeArguments = args.toList in
ClassManifestDeprecatedApis.scala was silently skipped.
Fix: Added DeclKind.Val and DeclKind.Var to the kind check, and added matching Val => "val " / Var => "var " cases
to declKeywordOnLine.
declKeywordOnLine and declLeadingKeyword used a greedy regex @[\w()\s,."]+ to strip annotations. Because \s
(whitespace) was included in the character class, replaceAll would consume @inline override as part of the
"annotation," stripping def along with it. (This only affected replaceAll ΓÇö findFirstMatchIn in DeclStartPattern
still worked via backtracking.)
Fix: Replaced the greedy replaceAll-based annotation stripping with calls to the existing
Declaration.dropLeadingAnnotations() helper, which properly handles annotation arguments with strings, balanced
parentheses, and nested brackets. Made dropLeadingAnnotations public (was private) for cross-file reuse.
The name-parsing loop in parseDef only accepted identifier-style characters (isLetterOrDigit, _, $). Symbolic
characters were rejected, producing an empty name "", which then failed the decl.name.nonEmpty check.
Fix: Added isSymbolChar() predicate for Scala operator characters (+, -, =, !, ?, :, ~, /, %, &, *, <, >, |, ^, ),
and modified parseDef to branch: if the name starts with a letter/digit/_/$, use identifier parsing; otherwise, use
symbol-name parsing.
Also updated DeclStartPattern regex with a more precise annotation-stripping regex (proper balanced bracket/paren
matching) for consistency.