Repository navigation
Conversation
A Scala def with a body is one entry. The decisions are if, while, do, for and its guard, each case of a match or a catch, a case guard, and each && and ||. A lambda and a local def count toward the enclosing method; an anonymous class body is skipped. The namespace is the package and the enclosing types, package.Outer.Inner, so uml-viewer attaches the score to that class; a top level def belongs to its package. An abstract def, a secondary constructor and a def with a missing name are not entries, and an operator def is named by its symbol. Coverage comes from sbt-scoverage's scoverage.xml, one per sbt module under target/scala-*/scoverage-report. Its statements become the same per line records LCOV fills, so a method is scored by its line range through the path already there. The runner is sbt coverage test coverageReport in the nearest directory with build.sbt. The ancestor walk and the enclosing name collector that Java, Python, Rust and TypeScript each spelled for themselves move to treesitter.py, so the Scala module is the first that does not add a copy. The chain of language checks in run_coverage becomes a table, which keeps it under CRAP 10 with a seventh language. Proved by: the suite, 155 passed and the one pre-existing babashka failure; crapper on itself, every changed function under CRAP 10 with fresh coverage; mutator on scala.py, treesitter.py and runners.py, every mutant killed; and a run on a Scala 3 project with its scoverage report, whose per method rates match the report. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An indented Scala 3 body parses as a block that runs on through the comments and the indentation after it, up to the next definition, so a def reported the first line of the following def as its end line and the line based coverage charged it with that def's statements. The Scala module now takes the end line from the last token of the def that is not a comment. The shared treesitter.end_line is unchanged; the other languages do not have this block shape. A new test in tests/test_scala.py covers an indented body, a braced body, trailing line and block comments, an end marker, and a top level def followed by a blank line. It failed first with w spanning 2 to 4 and passes now; the full suite is 156 passed, 1 deselected, and crapper at threshold 10 on scala.py exits 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
README promises that a run deletes the previous report of each language it measures, but the Scala runner left every scoverage.xml in place, so after a failed sbt run the loader scored Scala on the numbers of an earlier run instead of 0%. Before sbt runs in a module, the runner now removes each scoverage report directory the loader would read under that module. The loader's glob is now the public scoverage_paths, so the runner and the loader share one definition of where reports live. A new runner test writes stale reports at the module root and in a nested sbt module, makes sbt fail, and asserts both are gone. It failed before the change and passes after it; the full suite is green and crapper scores every function of both changed files under 10. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Why: README promises that Scala counts each case of a catch, but the
Scala 3 form `try 1 catch case e: E => 2` parses as a catch_clause that
holds a bare case token and no case_clause node, so the case was never
counted and the method scored complexity 1 instead of 2.
What: the Scala decision predicate also counts a catch_clause with a
direct case child. The braced form and the indented form with several
cases already parse into case_clause nodes and are unchanged, so that
catch_clause is counted once only when it holds its case directly.
Proof: test_a_brace_less_catch_case_is_a_decision failed with
[('f', 1)] before the change and passes after it; the full suite passes
and crapper at threshold 10 on scala.py exits 0.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The missing name check only caught a MISSING identifier, so `def [T] = 1`
became an entry named with the empty string and `def : Int = 1` became an
entry named Int, the return type taken as the name. The contract says a def
whose name is missing produces no entry.
The name is now the child right after the `def` keyword. The def is skipped
unless that child is a plain or operator identifier with no error inside,
which also covers the MISSING identifier of `def = 1`.
Proved by a new test with both defs that failed with [('', 1), ('Int', 1)]
and now passes; the full suite is green and every function in scala.py
scores under CRAP 10.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A def inside a given with a body took the namespace of the enclosing object, so two givens in one object produced entries with the same namespace and name, while scoverage writes each given as its own class (io.eezo.core.given_Parser_Boolean, io.eezo.core.option). uml-viewer could not tell them apart and coverage joined to the wrong class. The Scala name callback now recognises a given_definition with a with_template_body: a named given contributes its identifier, an anonymous one the name scalac synthesizes, given_ followed by the outer type name and the outer name of each type argument, so Ordering[Int] is given_Ordering_Int and Map[String, List[Int]] is given_Map_String_List. An alias given keeps the enclosing namespace, as scalac compiles it to a member of that object. ancestor_names accepts a name string from its callback for a name no node holds. README and the module docstring state the rule. Proved by a new test with five givens in one object, seen failing with every def in p.O before the change and passing after; the full suite is green and crapper scores every changed function under 10. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
typescript._class_names still copied the ancestor walk that treesitter.ancestor_names already owns, so the class name collector lived in two places. Java, Python and Rust already pass a name_node callback to ancestor_names; TypeScript now does the same. _class_names becomes _class_name, which returns the identifier of a class_declaration or abstract_class_declaration and None for any other node. The two class kinds live once in _CLASSES, which _SKIP reuses. A new test pins the namespaces first: an abstract class, a class nested in a method of it, an anonymous class expression whose method is dropped, and a JavaScript class. It passed before the change and after it. Dropping abstract_class_declaration from _CLASSES by hand makes it fail. The full suite passes with the babashka test deselected and crapper at threshold 10 exits 0 on typescript.py. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
No test had a do while, so removing do_while_expression from the Scala decisions left the suite green and such a loop scored one decision short. The SCORE fixture now holds a do while and expects 14. Proved by deleting do_while_expression from _DECISIONS by hand: the test failed with 13 instead of 14, and passed again once the mutant was reverted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every lambda in the Scala tests sat inside a def, so removing
lambda_expression from _EXECUTABLE left the suite green while a def in a
lambda in an object val initializer would have become its own entry.
A new test scores object O with val h = (x: Int) => { def inner = 1; inner }
and asserts no entry. With the mutant applied by hand it fails with
[('inner', 1)] == []; with the mutant reverted it passes, and the full
suite is green.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An infix such as `1 to n` or `a max b` names its operator with a plain identifier, so the Scala module finds no operator_identifier child. The guard that handles that case had no test: deleting it left the suite green while every such def would crash functions_in_source. The new test scores a def built from `to` and `max` and expects complexity 1. Proved by applying the mutant that drops the guard: the test fails with AttributeError; reverting the mutant makes it pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
No test had a local class, object or trait inside a def, so _skip could shrink to template_body alone and the suite stayed green, letting the decisions in a local class's parameters or extends clause count toward the enclosing method. The NESTED fixture now declares a local class with an if in a parameter default and another in its extends clause, and the test still expects outer to score 4. Proved by applying the mutant by hand (_skip returning only node.type == "template_body"): the test failed with outer scoring 6. Reverting it, the Scala tests and the full suite pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The only covered statement in the scoverage fixture had an invocation count of 2, so raising the threshold in parse_scoverage from > 0 to > 1 left the suite green, and a statement run exactly once would then have counted as uncovered. The fixture statement on line 4 now has an invocation count of 1. The expected rates do not change. Proved by applying the mutant by hand: tests/test_coverage.py had 11 passing before the fixture change and 2 failing after it, then all 11 passed again once the mutant was reverted. The full suite passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Only the report two directories down was tested, so dropping the root and one level globs in _scoverage_paths kept the suite green, and a single module sbt build could silently stop being read. test_load_bundle_reads_each_sbt_module_report now writes a report at the root, one directory down and two directories down, each for its own source file, and asserts the percentage of each. Proved by applying the reviewer's mutant (paths = [] in place of the root and one level globs) and, separately, dropping only the one level glob: the test failed both times, then passed again after reverting. Full suite: 155 passed, 1 deselected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
No test asserted either Scala coverage warning or where sbt runs, so deleting the "exited N" or the "No build.sbt" message, or running sbt in the scan root instead of the module, left the suite green. Two tests in tests/test_runners.py now cover the Scala runner. One puts build.sbt in a nested module, stubs run_shell to exit 3, and asserts the exact sbt argument vector, that it runs in the module directory, and the full "Scala coverage exited 3 in <module>" message. The other runs a Scala file with no build.sbt and asserts the skip message. Proved by hand: replacing each warning with pass, running sbt in root, and flipping the exit check each make one new test fail; reverting with git checkout makes them pass. The full suite is green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
No test placed a function inside an inline mod other than mod tests, whose functions are dropped, so replacing the ancestor module names in rust.py with an empty list left the whole suite green. Add a test with a function in mod outer, pub mod inner and assert its namespace is demo::outer::inner. Proved by applying the mutant by hand: the new test failed with demo instead of demo::outer::inner. Reverting the mutant made it pass, and the full suite is green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Replacing the _append_arrows guard enclosed_by(node, _FUNCTION_TYPES) with False left the whole suite green, because no test declared an arrow const inside another function. The new test declares an arrow const with a branch inside a function and asserts that only the outer function is an entry, carrying the branch in its complexity. Proved by applying the mutant by hand: the new test failed with inner reported as an extra entry. After reverting the mutant the test file and the full suite pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The _EXECUTABLE set in java.py could lose constructor_declaration and the suite stayed green, because no test put an anonymous class method inside a constructor. Add a test where a constructor declares an anonymous Runnable whose run method has a decision, and assert no entry results. Proved by removing constructor_declaration from _EXECUTABLE by hand: the new test failed with run emitted as an entry. Reverting the mutant made it pass, and the full suite is green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Scala paragraph of the README said only the cases of a match or a catch count, but the code counts every case clause, so a partial function literal passed to collect scores each of its cases too. The paragraph now names the partial function literal, and a new test pins it: collect with a guarded case and a wildcard case scores 4. The full suite passes with the crap4clj snapshot test deselected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The module docstring listed the languages its helpers serve but left out Scala, although scala.py now imports parse, complexity, enclosed_by and ancestor_names from it. Add Scala to that list. A docstring change carries no behavior, so no test applies; the full suite stays at 155 passed and crapper with threshold 10 on treesitter.py exits 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Scala change added two phrases to coverage.py that wrote "per-line records" in prose: the module docstring and the comment in load_bundle. The house rule forbids dash characters in prose, and the commit message of that change already wrote "per line records". Both phrases now read "per line records". The older Cloverage sentence on line 3 predates this branch and is left as it was. Only prose changed, so there is no new test; the full suite stays at 155 passed with the babashka test deselected, and crapper at threshold 10 on coverage.py exits 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Why
Scala projects built with sbt had no CRAP score. This adds Scala as the seventh language, with the same shape as the others: one entry per function, a namespace uml-viewer can attach it to, coverage read from the report the build already writes.
What
Entries. A
defwith a body is one entry. Decisions areif,while,do,forand its guard, eachcaseof amatch, acatchor a partial function literal (the brace less single catch case included), acaseguard, and each&&and||. A lambda and a local def count toward the enclosing method; an anonymous class body is skipped, as in Java. An abstract def, a secondary constructor and a def with no name are not entries. An operator def is named by its symbol. A def ends on its last body token, not in the indentation of the next line, which matters for line based coverage.Names. The namespace is the package and the enclosing types,
package.Outer.Inner, so uml-viewer attaches the score to that class. A top level def belongs to its package. A def in a given with a body belongs to the given, with scalac'sgiven_Type_Argname for an anonymous one.Coverage. sbt-scoverage's
scoverage.xml, one per sbt module undertarget/scala-*/scoverage-report, read at the root and up to two directories down. Its statements become the per line records LCOV already fills, so a method is scored by its line range through the path that exists. The runner issbt coverage test coverageReportin the nearest directory withbuild.sbt, and it deletes the previous report first so a failed run scores 0% as README promises.Shared helpers. The ancestor walk and the enclosing name collector that Java, Python, Rust and TypeScript each spelled for themselves now live in
treesitter.py, and all five modules use them. The chain of language checks inrun_coverageis a table.README and the pyproject description list Scala beside the others. No dependency changes: the grammar comes from
tree-sitter-language-pack.How to review
The first commit is the feature; the nineteen after it answer a review of it, one finding each, and every commit message says why, what changed and how it was proved.
src/crapper/languages/scala.pyandtests/test_scala.pycarry the rules above as tests.parse_scoverageandscoverage_pathsincoverage.pyare the reader;_cover_scalainrunners.pythe runner.How it was tested
tests/test_cli.py::test_project_snapshot_is_readable_as_crap4clj_ednwas deselected on the author's machine because babashka is not installed there; it is unchanged.parse_argsincli.pyis at 16 on master as well and is left alone.statement-rateentries, operator defs appear under their symbols, and the default runner refreshes every module's report.🤖 Generated with Claude Code