Repository navigation
fix(analyzers): a class's constructor is a component like any method - #149
Merged
Merged
Conversation
JavaScript (`constructor`), TypeScript (`constructor`) and Ruby (`initialize`) left constructors out of the graph on purpose; Java, C#, Kotlin (secondary constructors, `init` blocks) and Dart never emitted them. Python (`__init__`), PHP (`__construct`), C++ (`A.A`) and Scala (`A.this`) already did, as methods. A constructor often does the real work (node-semver's SemVer constructor parses the whole version string), so a tool that walks the graph's functions (documentation, or a review of a translation function by function) never saw it. In JS and Ruby it was still registered for call resolution, so calls in its body had a caller that was not a component and ast_parser dropped those edges; in Java, C#, Kotlin and Dart they went to the class. Now every one of these analyzers emits the constructor as a `method` component, named as the others are: `Class.constructor` (JS, TS), `Class.initialize` (Ruby), `Class.Class` (Java, C#, Dart's unnamed constructor; a record's compact constructor in Java), `Class.named` (Dart named and factory constructors), `Class.constructor` and `Class.init` (Kotlin). Calls in its body have it as caller. A Dart constructor with no code of its own (no body, no initializer list: `A(this.x);`, a redirecting factory) stays with its class, as before. Measured on bench repos (components / edges, before -> after): node-semver 104/110 -> 108/119 (4 constructors, 9 edges no longer dropped), superstruct 97/183 -> 99/185, rainbow 103 -> 108, semver4j 82 -> 90, semver (C#) 897 -> 920, qrcode (multi-language) 519/936 -> 540/937; cJSON, lz4, goja and tinyxml2 unchanged. Not covered: overloaded constructors share one id, as overloaded methods already do; Kotlin's and Scala's primary constructors have no body of their own (their code stays with the class).
…C# grammar
requirements.txt pins tree-sitter 0.23.2, whose Parser refuses tree-sitter-c-sharp 0.23.1's ABI 15
grammar ("Incompatible Language version 15. Must be between 13 and 14"), so the C# analyzer cannot run
at all under those pins; a fresh install from pyproject (tree-sitter 0.26) runs it. No C# test existed
before, so CI never showed it. The case is skipped where the grammar does not load, not hidden where it does.
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.
JavaScript (
constructor), TypeScript (constructor) and Ruby (initialize) left constructors out of thegraph on purpose; Java, C#, Kotlin (secondary constructors,
initblocks) and Dart never emitted them.Python (
__init__), PHP (__construct), C++ (A.A) and Scala (A.this) already did, as methods.A constructor often does the real work (node-semver's SemVer constructor parses the whole version
string), so a tool that walks the graph's functions (documentation, or a review of a translation
function by function) never saw it. In JS and Ruby it was still registered for call resolution, so calls
in its body had a caller that was not a component and ast_parser dropped those edges; in Java, C#,
Kotlin and Dart they went to the class.
Now every one of these analyzers emits the constructor as a
methodcomponent, named as the othersare:
Class.constructor(JS, TS),Class.initialize(Ruby),Class.Class(Java, C#, Dart's unnamedconstructor; a record's compact constructor in Java),
Class.named(Dart named and factoryconstructors),
Class.constructorandClass.init(Kotlin). Calls in its body have it as caller. A Dartconstructor with no code of its own (no body, no initializer list:
A(this.x);, a redirecting factory)stays with its class, as before.
Measured on bench repos (components / edges, before -> after): node-semver 104/110 -> 108/119 (4
constructors, 9 edges no longer dropped), superstruct 97/183 -> 99/185, rainbow 103 -> 108, semver4j
82 -> 90, semver (C#) 897 -> 920, qrcode (multi-language) 519/936 -> 540/937; cJSON, lz4, goja and
tinyxml2 unchanged.
Not covered: overloaded constructors share one id, as overloaded methods already do; Kotlin's and
Scala's primary constructors have no body of their own (their code stays with the class).
Found while testing: under the pins of requirements.txt (tree-sitter 0.23.2, tree-sitter-c-sharp 0.23.1),
Parser(Language(tree_sitter_c_sharp.language()))fails with "Incompatible Language version 15. Must be between 13 and 14", so the C# analyzer cannot run in CI's environment at all (a fresh install from pyproject gets tree-sitter 0.26 and runs it). No C# test existed before. The new C# case is skipped where the grammar does not load; the pin itself is left for a separate change.