Report the line number of each search result in TypeUses and MethodCalls - #8454
Open
knutwannheden wants to merge 1 commit into
Open
Report the line number of each search result in TypeUses and MethodCalls#8454knutwannheden wants to merge 1 commit into
TypeUses and MethodCalls#8454knutwannheden wants to merge 1 commit into
Conversation
…dCalls`
A caller running `FindTypes` or `FindMethods` learned which files matched and
what the matched code was, but not where it was, so acting on a row meant
grepping for a position the search had already computed.
`SearchResultRows` buffers the rows a search produces for one source file and
prints that file once with `PrintOutputCapture.MarkerPrinter.FENCED`, which
wraps each marked element in `{{markerId}}` after its prefix. A
`PrintOutputCapture` counts newlines as the file prints and places each fence as
it arrives, so one print positions every match at O(file) rather than O(matches
x file), and no printed text is retained. Files with no matches never print.
A search reports each element it matches once, tracked by element id, so that
two visit methods reaching the same element yield one row while a search running
after another that marked the same element still reports its own match. Since
`Markers.add` keeps out a `SearchResult` equal to one already present, and every
search result on an element fences in the same place, a row is positioned by
whichever marker the element carries.
Positions describe the source as printed when the search ran. When no position
can be established the column is left empty rather than guessed from the first
textual occurrence of the name, which would report matches on unrelated lines
that merely share a word.
Measured on `J.java` (6799 lines, ~190 `java.util.List` matches), the added cost
is +2.03ms (+19.8%) and +1.06MB against a search-only recipe run. Removing all
~190 per-match `printTrimmed` calls was measured to save only 95KB and <=0.4ms,
so the whole-file print is additive here rather than a replacement: matches are
type names of a few nodes each.
`FindTypesBenchmark` gains a dense-file scenario. It locates its input by
walking up from the working directory, since `JavaCompilationUnitState` resolves
the repository root through `Class.getResource`, which is null once the JMH
plugin runs from its shaded jar.
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
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.
Motivation
A caller who runs
FindTypesorFindMethodslearns which files matched and what the matched code was, but not where it was.TypeUses.Rowrecords what matched, not where, so acting on a row means going back and grepping for a position the search had already computed and thrown away. In measured agent runs, 45 of 48 file reads were preceded by a grep whose only purpose was recovering that line number.SourcePositionService.positionOfalready returns absolute line numbers, but it prints from the compilation unit per element, so using it per match would be O(matches × file). This takes the approach used by moderne-cli'sSearchIndexinginstead: tag the matches, print the file once, recover every position from that one pass.Examples
TypeUsesandMethodCallseach gain aLine numbercolumn, appended last so positional consumers of the CSV keep working:A search populates it by handing
SearchResultRowsthe row it wants, in place of callingSearchResult.found:Summary
SearchResultRows(new,rewrite-core) buffers the rows a search produces for one source file, then prints that file once withPrintOutputCapture.MarkerPrinter.FENCED, which wraps each marked element in{{markerId}}after its prefix. APrintOutputCapturecounts newlines as the file prints and places each fence as it arrives, so one print positions every match and no printed text is retained. Files with no matches never print.TypeUses.RowandMethodCalls.Rowgain a nullablelinecolumn.FindTypes,FindMethods,FindDeprecatedMethods, andFindDistinctMethodspopulate it — all four recipes that write these two tables.Markers.addkeeps out aSearchResultequal to one already present, and every search result on an element fences in the same place, a row is positioned by whichever marker the element carries.FindTypesBenchmarkgains a dense-file scenario, which locates its input by walking up from the working directory.JavaCompilationUnitStateresolves the repository root throughClass.getResource, which is null once the JMH plugin runs from its shaded jar, so the benchmarks using that state currently fail in warmup onmain.Cost
Measured on
J.java(6799 lines, ~190java.util.Listmatches) with 3 forks × 10 iterations, the added cost against a search-only recipe run is +2.03ms (+19.8%) and +1.06MB (+13.2%). That is the worst case: one print per matched file, nothing for unmatched files, andUsesTypealready skips most files.Removing all ~190 per-match
printTrimmedcalls was measured to save only 95KB and ≤0.4ms, so the whole-file print is additive here rather than a replacement —TypeUsesmatches are type names of a few nodes each, unlike the declarations moderne-cli anchors on. The remaining cost is the print traversal itself, where the per-nodeCursorallocation inTreeVisitor.visitis the lever; that would speed up all printing and belongs in its own change.Test plan
FindTypesTest.lineNumberOfEachMatch— each match gets its own line, and a javadoc mentioning the type name in prose does not attract a position, so newlines inside comments are countedFindMethodsTest.lineNumberOfEachMatch— invocation, a call split across lines following a three-line comment, and a call inside a lambdaFindMethodsTest.reportsAMatchAlreadyMarkedByAnotherSearch— two overlapping searches over one call each report their own row, at the right line; verified failing before the fixFindTypesTest.simpleName,FindTypesTest.dataTable,FindMethodsTest.datatableFormat,FindDistinctMethodsTest.markFirstOccurrenceOfMethod— existing row and CSV assertions updated with expected lines./gradlew :rewrite-core:test :rewrite-java:test :rewrite-java-test:test./gradlew licenseFormat(no changes) and:rewrite-benchmarks:jmhClasses