Repository navigation
Improve static analysis and CLI - #2
Merged
Merged
Conversation
.yardoc is a build artifact: `rake yard` regenerates it, and every doc build dirtied five tracked files, three of them binary. The cache also stores its own copy of every doc comment, so the repository carried a stale duplicate of the sources that no source grep would ever reach. Ignored now; the files stay on disk.
A scan of a few thousand files went several seconds with nothing on screen, and stdout has to stay pipeable, so the narration goes to stderr: one line per phase, rewritten in place on a terminal, `◆ Audition checking 50/919 5% (0.2s, on 8 ractors)`. A phase running in Ractors says how many, next to the clock rather than the counts, because the counts move too fast to read. On above 200 files and for every bundle sweep, off for `--format json` and `--format github`, and `--progress` / `--no-progress` force either way. Progress::SILENT is the default everywhere, so a caller that wants nothing passes nothing.
Contiguous slices put neighbouring files on one worker, and neighbours in a tree are alike in size, so one worker could draw a slice weighing several times the mean and still be running once the rest had finished. Files are now dealt heaviest-first onto whichever worker is carrying least (longest-processing-time-first, which finishes within 4/3 of an optimal split). 2,500 files went 2.3s to 1.1s on 8 workers. The worker count is every core the Ractor pool can actually run, `Etc.nprocessors` capped by `RUBY_MAX_CPU`, where it was one less than the core count: Ractors past that cap add no parallelism. `-j` / `--workers` still overrides.
The four whole-tree walks ran serially after the per-file pass. They now run in the same Ractors above 100 files: each worker parses its slice once and keeps the trees, one round reports the class and module names its slice declares, and once those are merged a second round walks the same trees against them. 3,400 files went 2.4s to 1.2s on 8 workers, the whole phase 3.8s to 2.6s. Below the threshold the serial path still runs, and says so under `-w`. The same pass gained the `static-scan` check, for the walks' own blind spots. rubydex reports every expression it could not resolve; where the shape could hide what the class-state and constant walks look for, the hole is now a finding instead of a clean line: a singleton opened on a runtime receiver that writes class-level state, a runtime superclass or mixin argument, a constant assigned through an unresolved path. Only class and module bodies are read, so an `include` matcher in a spec stays silent, and a line another check already reported keeps its own finding.
A call into a C extension that never declared rb_ext_ractor_safe raises Ractor::UnsafeError, and the extension is outside the tree, so the call site is what can be flagged. Which gems count comes from the target's own Gemfile.lock, its installed extension binaries and its generated type stubs, never a gem list, and a platform build Audition cannot inspect is reported as unverified rather than assumed either way. Taint then follows the values: chained calls, locals and ivars, sig/T.let/T.cast types, case/when and is_a? narrowing, block element parameters, arguments into other methods, reopened classes, mixin bodies, and across files until nothing new is learned. Two checks come with it: dependency-class-state, for class-level state a stub attributes to a dependency, and unshareable-reads, for reads of a constant another gem froze nothing into.
A freeze can cover only the surface of its value, and the classifier took the freeze at its word. `X.new.freeze`, `compute.freeze`, and frozen containers of fresh instances or unprovable call results now report as shallow_opaque: the wrapper is frozen, the value is not. A later bare `NAME.freeze` counts the same, and one unknown element no longer excuses a provably fresh sibling. Indexing another call's result is an unprovable call result too, and a constant-read key does not make an opaque value provable. Memoization frozen at the top is an info note rather than an error, since nothing can mutate through it.
Color stopped at the findings list. The bundle sweep gained a titled table with severity glyphs on the verdicts, one color per row and blank cells where a count is zero; usage text, the baseline line, fix and dry-run output, `--compare` deltas and `--capabilities` follow the same palette. `--plain`, `NO_COLOR` and `TERM=dumb` strip all of it. A finding now carries whether it came from the target's tests. Test findings render with a `(tests)` mark and are summarized on their own line, because a frozen constant in a spec is not the same debt as one on a production boot path.
`test`, `spec` and `features` were a constant, so a project that files its tests anywhere else had every test finding rated as production code. `test_dirs` in .audition.yml replaces that list; absent, the default stands. It replaces rather than extends, an explicit empty list is honored as "no test directories", and the `_test.rb` / `_spec.rb` suffixes count whatever it says. Entries are validated as single path segments, since each is matched against one path component and a nested path would silently match nothing.
Generated type stubs were read from a fixed `sorbet/rbi/gems`, so a project whose generator writes them anywhere else silently lost the compiled-extension evidence those stubs carry. The extension is the convention; where the generator writes them is not. Stubs are now globbed as `*.rbi` anywhere in the target, and the gem-calls check matches `<name>@<version>.rbi` by basename within that set. Native extensions were globbed from `ext/*`, which is also only a habit. They are found by the build file that actually compiles them (`extconf.rb`, `Cargo.toml`, `build.zig`) wherever it sits, and a workspace manifest above a crate is dropped so the crate's sources are not reported twice. A gem checkout that keeps its code outside `lib` is scanned whole rather than not at all. Reading require_paths off the gemspec would mean evaluating it, which the static pass never does, so `lib` remains the default with the checkout as the fallback. Excluded and hidden trees are skipped throughout, as before.
The require probe's entry-file fallback globbed `lib` under the target root, which is the packaging default and nothing more. It globs the load paths the payload already carries and the harness already unshifts onto $LOAD_PATH, so a target that keeps its code elsewhere gets the same fallback. Installed-gem targets and sweep rows now carry their full require paths. The probes also extend the native-extension check to dependencies: the harness records every compiled file the load pulled in and byte-scans those too, reporting silent ones as `runtime-native-extension` warnings attributed to the gem that shipped them, and skipping what the static check already covered.
The CLI and the bundle sweep gain the pieces the preceding commits added: the gem-calls pass over the target's stubs, the graph audit's worker count, progress phases named for what a waiting reader wants to know, and the configured test directories at the one site that tags a finding as test code. Stub and compiled-file lists pass through `exclude` the same way the Ruby file list does. Per-gem scanning in a sweep stays serial: worker threads already parallelize across gems, and a Ractor storm underneath them would cost more than it buys.
The design doc described two probes; there are three passes, and the Ractor mechanics the scanner depends on were written down nowhere. It now covers the fan-out, the work split, the thresholds, and the six Ruby 4.0.6 facts the scanner leans on, with the component table brought up to date and the out-of-scope list corrected where features had since shipped. The README documents `static-scan`, the stub and extension discovery, and the `test_dirs` key. The knowledge base and the agent skill drop a directory convention that is no longer assumed, and the self-audit job is named for what it does.
Class-level state was found in a module only where the target's own source spelled the extend. A concern never spells it: the mixin that puts its companion module on the class lives in whatever library defines the pattern, so a nested `ClassMethods` module and a `class_methods do` block both went unread, and the ivars their methods write were reported clean. Which trees reported the shape turned on an unrelated file. One literal `extend ClassMethods` anywhere in a scan seeded the name for the whole of it, so identical code was an error in a library that happened to contain one and silent in a library that did not. The name is seeded outright now: inside the target it is the only evidence there is. `extend const_get(:Name)` on self, and a module mixed into a `singleton_class`, join the extend sites the walk already reads. A mixin argument that resolves either way stops being reported as one the graph could not read, which would otherwise contradict the finding the walk emits on the same line. Report-only, as the shape already was. A lazy memo cannot be precomputed at load time, so no rewriter offers to.
The graph audit's static scan asks each diagnostic's rule for its name, and the spec that guards the rule table reads Rubydex::Rules. Both arrived with rubydex 0.4: 0.3.0 hands back a bare Symbol and ships no Rules module, so every scan raised there while the floor of 0.2 promised it would run. The floor now says what the code needs.
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.
No description provided.