Repository navigation
feat(upstream): catch up to 60b0d7a9 — Spring routes and affected filter globs - #589
Conversation
…ge-private one (colbymchenry#3009) The Spring extractor found a mapping's handler with a pattern that wanted `public`, `private` or `protected` (or Kotlin's `fun`) before the name, in the next 600 characters. A package-private handler, like Spring Boot 3's `@GetMapping List<Article> list()`, matched nothing, so its route linked the next method declared with a modifier, or nothing at all, and a method-level `@RequestMapping(value = "/feed", method = GET)` on such a method made no route. A generic Kotlin `fun <T> find()` handler fell through the same way. Separately, a route bound by name alone took the wrong overload when a tall annotation stack (`@Operation`, `@ApiResponses`) above the mapping left the next overload's declaration nearer the annotation. The extractor now reads the declaration the annotation is written on: past it and every annotation stacked with it (arguments read as balanced groups, strings and text blocks included), the modifiers (none at all for a package-private method), type parameters and the return type (void, a primitive, a qualified or nested generic type, an array) to `name(`; for Kotlin, past its modifiers to `fun`, type parameters and `name(`. A field, a constructor or a class under the annotation gives no handler. The mapping annotation's own arguments are read the same balanced way. At resolution, the route's handler reference binds to the method of that name whose span (which starts at its first annotation) holds the annotation's line, the innermost such method; any other case resolves by name as before. Validation, kernel engine, main 01e0ac3 vs this commit: - realworld-java21 93e018e: Spring routes linked 4/19 -> 19/19; the 15 new edges each go to the method the mapping is written on. - spring-petclinic 500158f (17 routes), spring-petclinic-kotlin 817dba3 (18), eladmin 55fbf70 (133), mall dcaa93b (246), dolphinscheduler 21968bc (239): same targets, only the edge metadata changes (exact-match 0.7/0.9 -> framework 0.95). - Controls jsoup 0c508f1 (Java, no Spring), requests 3c7baaa (Python), cobra adbc881 (Go): dump-graph byte-identical. - Index time, median of 3 interleaved: mall 12.3 s vs 12.6 s, eladmin 3.02 s vs 3.03 s (noise on a shared machine). - New tests fail on main's java.ts (20 failing) and pass with the fix, in both engines; dropping the positional binding fails only the overload test. Reimplements PR colbymchenry#851 by @Liurchen, who found the package-private gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3d19da2)
…oundary (colbymchenry#3029) `codegraph affected --filter` turned its glob into an unanchored regex by hand (`**` to `.+`, `*` to `[^/]*`, no `^` or `$`), so it matched anywhere inside a path: `tests/**/*.py` missed `tests/x.py` (`.+` needs a folder), a changed `tests/x.py.bak` counted as a test under `tests/*.py`, the changed test itself was missed under `tests/**/*.py`, `e2e/*.spec.ts` took an `xe2e/` folder, and `/tests/*.py` matched only `other/tests/...`. The filter is now a picomatch glob (an existing dependency, loaded only by this command) with `dot: true`, matched against the whole project-relative path. A pattern with no slash matches the file name at any depth. One with a slash matches from any folder boundary, an implicit leading `**/`, which keeps the help text's own `e2e/*.spec.ts` matching `apps/web/e2e/...` as the filter always did; a leading `/` or `./` anchors it at the project root, and a trailing `/` takes everything under that folder. A pattern picomatch rejects is reported as an invalid --filter instead of failing later. Validation: new CLI-level cli-affected-filter-globs tests (6): 4 fail on main's source and pass with the fix; the other 2 guard behavior kept on purpose (file-name patterns at any depth, folders outside the test conventions, dotfiles, a trailing-slash folder). The existing affected CLI suites pass. The triage fixture shows the same before and after. Reimplements PR colbymchenry#1273 by @Jseca666. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 60b0d7a)
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
![bompus-ai-review[bot]](https://avatars.githubusercontent.com/u/253635?s=60&v=4){kind=small}
| i = k + part.length; | ||
| } else if (s[j] === '[' && s[skipSpace(s, j + 1)] === ']') { | ||
| i = skipSpace(s, j + 1) + 1; | ||
| } else { |
There was a problem hiding this comment.
medium: Preserve handlers with annotations on array return types
The return-type loop handles '<', '.' and '[]', then executes 'else { break; }'. For an annotation before an array dimension, the subsequent 'readIdentifier(s, i)' encounters '@' and returns null.
Failure: A valid handler such as '@RequestMapping(value="/x", method=RequestMethod.GET) public String @nullable [] list()' (using a TYPE_USE Nullable annotation) now produces no route. With @GetMapping, its route loses its handler reference. The previous method search accepted this declaration.
Suggested fix: Consume type-use annotations while parsing array dimensions and add regression coverage for both mapping forms.
There was a problem hiding this comment.
This traces the pre-fix loop. d5b01c4 adds the annotation branch at src/resolution/frameworks/java.ts:714 (if (s[j] === "@") { ... i = annotation.end; }) before the </./[] branches. The exact case is covered in __tests__/spring-route-handlers.test.ts (public String @Nullable [] names() and a method-level @RequestMapping(value = "/feed", ...) returning String @Nullable []), and passes on both the native and wasm backends (Tests 2 passed). Please re-check against that revision.
…nry#3009 review) `annotatedMethodName` stopped at a type-use annotation in a qualified or array return type, so `public String @nullable [] names()` and `public java.util.@nullable List<String> names()` lost their route's handler link. Read annotations at each return-type position and add both declarations, plus a method-level `@RequestMapping`, to the Spring test. Also correct two copied [Unreleased] bullets that claimed the CLI read commands open the index read-only and report a rebuild: the fork's CLI still opens read-write, so the claims did not describe this fork. Document the affected filter's whole-name, folder-boundary, root-anchor and trailing-slash matching in the README and the site guide.
`java.util.@nullable @readonly List<String>` left the second annotation unread, so the method name lookup hit `@` and the route lost its handler. Consume every consecutive annotation after the dot, cover both a `@GetMapping` and a method-level `@RequestMapping` handler, and add the missing "no slash matches the file name" rule to the README's affected filter description.
| /** Words that begin a declaration other than a method where a Java return type would stand. */ | ||
| const NOT_A_RETURN_TYPE = new Set(['class', 'interface', 'enum', 'record']); | ||
|
|
||
| const IDENTIFIER = /[A-Za-z_$][\w$]*/y; |
There was a problem hiding this comment.
medium: Preserve handlers with Unicode return-type names
const IDENTIFIER = /[A-Za-z_$][\w$]*/y only reads ASCII identifiers. Parsing Réponse stops after R, so annotatedMethodName returns null; method-level RequestMapping extraction then executes if (!handler) continue.
Failure: A valid handler @RequestMapping(value="/feed", method=RequestMethod.GET) public Réponse feed() loses its route entirely. With @GetMapping("/feed"), the route loses its handler reference. Before/after extraction probes confirm both regressions.
Suggested fix: Accept Unicode Java identifiers in return types and their qualifiers, and cover a public handler returning a Unicode-named DTO.
| if (i < 0) return null; | ||
| } else if (s[j] === '.') { | ||
| let k = skipSpace(s, j + 1); | ||
| if (s[k] === '@') { |
There was a problem hiding this comment.
medium: Consume every type-use annotation after a qualifier
The qualifier branch uses if (s[k] === '@') to consume one annotation, then immediately calls readIdentifier(s, k). A second annotation leaves k at @, causing the parser to return null.
Failure: For valid TYPE_USE annotations A and B, @RequestMapping(value="/feed", method=RequestMethod.GET) public java.util.@A @B List<String> feed() previously produced a route and handler reference; the current extractor produces neither. With GetMapping, the route remains disconnected from its handler.
Suggested fix: Replace the single-annotation condition after the dot with a loop that consumes all consecutive annotations before reading the type name, and add regression coverage.
What changed
Catch the fork up to upstream
mainat60b0d7a9. The first 41 of the 43 commits since the fork's69af19acmerge point were already ported in #586/#587; the two still missing are cherry-picked here:3d19da2f— a Spring route links the method its mapping annotation is written on, including a package-private handler and generic Kotlinfun, and an overload under a tall annotation stack.60b0d7a9—codegraph affected --filtermatches whole names from any folder boundary (picomatch; leading/anchors at the project root, trailing/takes the folder).Only one conflict:
src/bin/codegraph.tsstill opens the index through the fork's lazyloadCodeGraph()+CodeGraph.open(), where upstream usesopenIndexToRead(). The fork's loader is kept and the filter is compiled before the index opens, as upstream intends.README.mdnow names60b0d7a9as the upstream point, and thepetclinicprecision report is updated.Tests
spring-route-handlers,cli-affected-filter-globs) plusframeworks,cli-affected-pathsandcli-affected-test-conventions— all pass.eval:precision -- petclinic: 1/1 present held.60b0d7a9follows in a separate report; this PR is the code catch-up.Review fixes
annotatedMethodNamenow reads type-use annotations inside qualified and array return types, including consecutive ones (public java.util.@Nullable @NonNull List<String> names()), with both a@GetMappingand a method-level@RequestMappinghandler covered by the Spring test.[Unreleased]bullets claiming the CLI read commands open the index read-only and report a rebuild. The fork's CLI still opens read-write; the claims did not describe this fork.