fix: resolve bash sources for extensionless libs and bare names - #2192
Closed
Souptik96 wants to merge 1 commit into
Closed
fix: resolve bash sources for extensionless libs and bare names#2192Souptik96 wants to merge 1 commit into
Souptik96 wants to merge 1 commit into
Conversation
…hify-Labs#2171) Two coverage gaps left by Graphify-Labs#2141 / Graphify-Labs#2157, both misses rather than fabrications. 1. Extensionless shebang scripts. _SHEBANG_DISPATCH already routes a `#!/usr/bin/env bash` file with no extension to extract_bash, so its functions are indexed, but the cross-file source-resolution pass picked participants by filename suffix (`p.suffix in (".sh", ".bash")`). A sourced extensionless lib was therefore excluded and calls into it never bound. Select by shape as well: the bash extractor tags every node it emits with metadata.language == "bash". The suffix check stays so an empty .sh file, which has no nodes to inspect, still participates. 2. Bare `source lib.sh` (no ./ prefix). Only the `raw.startswith((".", "/"))` branch recorded a bash_sources entry; a bare name fell through to the opaque `imports` fallback, so neither the source edge nor calls into the lib resolved even though the file usually sits beside the script. The else branch now binds a sibling of that name when one exists. The existence gate from the ./-prefixed branch carries over: a bare name that resolves to no sibling keeps the old `imports` edge and records no bash_sources entry, so nothing is invented. is_file() is wrapped against OSError so a name that is invalid for the platform degrades instead of raising. Transitive sources (a->b->c) remain unresolved, as the issue notes.
Collaborator
|
Thanks @Souptik96. Shipped in v0.9.27 (cherry-picked to v8), with a follow-up marking the bare-name binding INFERRED (it resolves via $PATH at runtime). Closed-unmerged here, but it's in the release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.27 |
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.
Fixes #2171
Both gaps confirmed against
v8(0.9.26) and fixed along the directions in the issue.1. Extensionless shebang scripts
_SHEBANG_DISPATCHroutes a#!/usr/bin/env bashfile with no extension toextract_bash, so its functions are indexed — but the cross-file source pass pickedparticipants by filename suffix:
so a sourced extensionless lib was excluded and calls into it never bound.
Now selected by shape as well: the bash extractor tags every node it emits with
metadata.language == "bash". I kept the suffix check alongside it so an empty.shfile— which has no nodes to inspect — still participates.
2. Bare
source lib.shOnly the
raw.startswith((".", "/"))branch recorded abash_sourcesentry; a bare namefell through to the opaque
importsfallback, so neither the source edge nor calls into thelib resolved. The
elsebranch now binds a sibling of that name when one exists.The existence gate from the
./-prefixed branch carries over, so this stays a miss-fix andnot a fabrication: a bare name resolving to no sibling keeps the old
importsedge andrecords no
bash_sourcesentry.is_file()is wrapped againstOSErrorso a name that'sinvalid for the platform degrades instead of raising.
Transitive sources (a→b→c) remain unresolved, as the issue specifies.
Testing
Three new tests in
tests/test_extract.py— the two the issue asks for, plus a guard thatthe bare-name branch still fabricates nothing:
Confirmed they catch the bugs — same tests with
graphify/extract.pyandgraphify/extractors/bash.pyreverted tov8:Affected suites and both pre-commit gates:
Those 4 failures are pre-existing on Windows, not from this PR — they're exactly the 4
test_extract.pyfailures in my baseline on unmodifiedv8(66d8110), all symlink-related(
test_collect_files_*, e.g.OSError: [WinError 1314] A required privilege is not held by the client). Full-suite baseline there is 45 failed / 3578 passed / 15 skipped. NoLinux/macOS box here to confirm they're green, so please read that in context.