fix: Sonar follow-ups — ModuleDeps ordering, Express dead code, RepositoryIdentity env#60
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| */ | ||
| private static void requireGit() { | ||
| try { | ||
| Process p = new ProcessBuilder("git", "--version") |
The dispatch chain in detect() checked `.endsWith(".gradle")` before the
settings-specific branch, so any `settings.gradle` / `settings.gradle.kts`
path routed to detectGradle() and the specialised detectGradleSettings()
helper was never reached. Gradle multi-module `include ':foo'` entries
were silently lost.
Reordered the dispatch so settings files are matched first, and added
ModuleDepsDetectorTest to lock in the reachability contract (plus a
regression guard that `build.gradle` still routes to detectGradle).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AbstractTypeScriptDetector#detect() unconditionally dispatches to detectWithRegex, and the class is annotated @DetectorInfo(parser = REGEX), so the protected detectWithAst override (and its three private ANTLR helper methods: extractIdentifierText, extractFirstStringArg, extractStringLiteral) were never invoked. Removed the AST method + helpers and their now-orphaned ANTLR / JavaScriptParser imports. The Python detectors define their own extractFirstStringArg with a distinct signature — unaffected. All 28 ExpressRoute* tests remain green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The two git-backed tests (resolve_gitRepoWithCommit_commitShaPresent,
resolve_detachedHead_branchIsNull) failed when the developer's global
gitconfig forced commit signing (commit.gpgsign=true, signingkey set) on
a machine without a usable signing key — git commit exited non-zero, the
original run() helper ignored the exit code, no commit was made, and
rev-parse HEAD returned null.
Made the git invocations hermetic:
* repo-local config overrides commit.gpgsign / tag.gpgsign to false,
unsets core.hooksPath, core.autocrlf, init.templateDir
* explicit --no-gpg-sign on the commit (belt-and-braces)
* scrub GIT_* env vars on every child process so no ambient CI /
worktree state leaks in
* run() now asserts the process exited 0 — silent failures become
loud test failures
* new requireGit() uses Assumptions.assumeTrue to skip cleanly when
the git binary is absent (product still covered by non-git tests +
RepositoryIdentity's own swallow-on-error path)
Verified by running the pre-fix test against a hostile HOME with forced
GPG signing — reproduces the 2 failures. Post-fix: 8/8 pass under the
same hostile environment.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aksOps
force-pushed
the
fix/sonar-followups
branch
from
92fd0de to
5646a92
Compare
|
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.
Changes (one commit each)
fix(detector): ModuleDepsDetector reaches settings.gradle branch
Real bug —
.endsWith(".gradle")matchedsettings.gradlefirst, shadowingdetectGradleSettings. Gradle multi-moduleinclude ':foo'entries were silently lost. Reordered the dispatch chain so the settings branch is evaluated before the generic.gradlebranch. AddedModuleDepsDetectorTest(9 tests) to lock in the reachability contract plus a regression guard thatbuild.gradlestill routes todetectGradle.refactor(detector): remove dead detectWithAst from ExpressRouteDetector
AbstractTypeScriptDetector#detect()unconditionally dispatches todetectWithRegex, and the detector is annotated@DetectorInfo(parser = REGEX), so the protecteddetectWithAstoverride (and its three private ANTLR helpers:extractIdentifierText,extractFirstStringArg,extractStringLiteral) was never invoked. Deleted the dead method, helpers, and the now-orphaned ANTLR /JavaScriptParserimports. All 28 existingExpressRoute*tests stay green. (Verified the Python detectors' ownextractFirstStringArghas a distinct signature and is unaffected.)fix(test): RepositoryIdentityTest no longer depends on local git state
Root cause: the two failing tests (
resolve_gitRepoWithCommit_commitShaPresent,resolve_detachedHead_branchIsNull) inherit the developer's globalcommit.gpgsign=trueand fail silently when no signing key is available — therun()helper was ignoring non-zero exit codes, sogit commitproduced no commit andrev-parse HEADreturned null. Fix: inject repo-local config that disables GPG signing, hooks, autocrlf, and template dir; add--no-gpg-signon the commit; scrubGIT_*env vars on every child; assert non-zero exit codes loudly (silent failures become loud failures);assumeTrueskip whengitbinary is absent. Verified by running the pre-fix test under a hostile HOME with forced GPG signing — reproduces the exact 2 failures; post-fix: 8/8 pass.(skipped) chore: remove 4 unused private fields
No-op on current
main:MODEL_INIT_RE,BATCH_MAPPING_RE,PROP_REQUESTMAPPING, andENTITY_REwere already removed by merged PR #58 (chore(sonar): clean unused imports, suppressions, and private members, commiteaea1ff). Verified viagrepinsrc/main— zero hits. No commit needed.Test count: 3294 → 3303, all green (31 pre-existing env-gated E2E skips).