Skip to content

Commit 44223d8

Browse files
quality: cut dead-code false positives across C++, C#, XAML, tests (#190)
Five complementary fixes targeting the dominant FP buckets observed in the wild on desktop .NET / Win32 repositories. - unused_export now treats an incoming `calls` / `method_implements` / `reads` edge on the symbol itself as evidence of life. The existing pass only checked file-level `imported_names`, which missed intra-file C++ helpers and qualified `Foo::method` definitions reached via call resolution rather than headers. - C++ qualified definitions (`void Foo::method() { … }`) now extract with `parent_name=Foo` and `kind=method`. New cpp.scm pattern for two-level `NS::Foo::method` declarations plus a parser helper that walks the qualified_identifier scope to recover the immediate enclosing type. Free functions are unaffected. - `dynamic_hints/dotnet.py` learned `typeof(TypeName)` — catches `[JsonConverter(typeof(X))]`, `[TypeConverter(...)]`, `DataTemplate.DataType = typeof(X)`, and manual DI registration. - `dynamic_hints/xaml.py` learned `<prefix:TypeName …>` element-tag references — catches converters, controls, templates declared as XAML elements without a `using` directive on the C# side. Property syntax (`<Grid.Resources>`) and bare built-in tags are excluded. - `_NEVER_FLAG_PATTERNS` adds standard test-project globs (`*Tests/*.cs`, `*.UnitTests/*.cs`, `*FuzzTests/*.cs`, `*UITest*/*.cs`, `*Tests.cs`, …) plus `*/unittests/*.cpp|.h`.
1 parent 1a16541 commit 44223d8

11 files changed

Lines changed: 266 additions & 7 deletions

File tree

packages/core/src/repowise/core/analysis/dead_code/analyzer.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,14 +372,17 @@ def _detect_unused_exports(
372372
if self._should_never_flag(str(node), whitelist):
373373
continue
374374

375-
symbols = [
376-
self.graph.nodes[succ]
375+
# Pair each symbol's data with its node id so we can check
376+
# incoming ``calls`` edges on the symbol itself further down.
377+
symbol_pairs = [
378+
(succ, self.graph.nodes[succ])
377379
for succ in self.graph.successors(node)
378380
if self.graph.nodes[succ].get("node_type") == "symbol"
379381
and self.graph.get_edge_data(node, succ, {}).get("edge_type") == "defines"
380382
]
381-
if not symbols:
383+
if not symbol_pairs:
382384
continue
385+
symbols = [sym for _, sym in symbol_pairs]
383386

384387
file_has_importers = self.graph.in_degree(node) > 0
385388

@@ -408,7 +411,7 @@ def _detect_unused_exports(
408411
and sym.get("end_line", 0) > sym.get("start_line", 0)
409412
]
410413

411-
for sym in symbols:
414+
for sym_id, sym in symbol_pairs:
412415
if sym.get("visibility") != "public":
413416
continue
414417
sym_name = sym.get("name", "")
@@ -476,6 +479,20 @@ def _detect_unused_exports(
476479
if has_importers:
477480
continue
478481

482+
# Symbol-level usage signal: any incoming ``calls`` /
483+
# ``method_implements`` / ``reads`` edge means somewhere
484+
# in the codebase actually uses this symbol — even if
485+
# the file-level ``imported_names`` machinery missed it
486+
# (intra-file C++ helpers, ``Foo::method`` qualified
487+
# definitions linked by call resolution but never named
488+
# in a header, Razor/XAML code-behind dispatches).
489+
if self.graph.has_node(sym_id) and any(
490+
self.graph[pred][sym_id].get("edge_type")
491+
in ("calls", "method_implements", "reads")
492+
for pred in self.graph.predecessors(sym_id)
493+
):
494+
continue
495+
479496
if is_deprecated:
480497
confidence = 0.3
481498
elif file_has_importers:

packages/core/src/repowise/core/analysis/dead_code/constants.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,28 @@
120120
"*/Themes/*.xaml",
121121
"*/Styles/*.xaml",
122122
"*/Resources/*.xaml",
123+
# ---- Test infrastructure conventions -----------------------------
124+
# Test classes are loaded by the test runner via reflection on
125+
# ``[Test]`` / ``[TestMethod]`` / ``[Fact]`` attributes — they
126+
# never appear in a `using` import that names the class. Match
127+
# both the file location *and* the standard suffix patterns so we
128+
# catch tests dropped at arbitrary paths.
129+
"*Tests/*.cs",
130+
"*.Tests/*.cs",
131+
"*UnitTests/*.cs",
132+
"*.UnitTests/*.cs",
133+
"*IntegrationTests/*.cs",
134+
"*.IntegrationTests/*.cs",
135+
"*FuzzTests/*.cs",
136+
"*.FuzzTests/*.cs",
137+
"*UITests/*.cs",
138+
"*.UITests/*.cs",
139+
"*UITest/*.cs",
140+
"*UITestAutomation/*.cs",
141+
"*/unittests/*.cpp",
142+
"*/unittests/*.h",
143+
"*Tests.cs",
144+
"*UnitTests.cs",
123145
)
124146

125147
# Decorator patterns that indicate framework usage (route handlers, fixtures, etc.)

packages/core/src/repowise/core/ingestion/dynamic_hints/dotnet.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@
7777
# resolves: ``_files_for`` strips the namespace.
7878
_NAMEOF_TYPE_RE = re.compile(r"\bnameof\s*\(\s*([A-Z][\w.]*)\s*\)")
7979

80+
# ``typeof(TypeName)`` — used heavily in ``[JsonConverter(typeof(X))]``,
81+
# ``[TypeConverter(typeof(X))]``, ``DataTemplate.DataType = typeof(X)``,
82+
# ``services.AddSingleton(typeof(IFoo), typeof(FooImpl))``, route
83+
# constraints, etc. Same shape and PascalCase guard as ``nameof``.
84+
_TYPEOF_TYPE_RE = re.compile(r"\btypeof\s*\(\s*([A-Z][\w.]*)\s*\)")
85+
8086

8187
class DotNetDynamicHints(DynamicHintExtractor):
8288
"""Discover DI registrations, reflection, and assembly-level hints in .NET."""
@@ -273,6 +279,20 @@ def _files_for(name: str) -> list[str]:
273279
)
274280
)
275281

282+
# ---- typeof(TypeName) — JsonConverter/TypeConverter attrs,
283+
# DataTemplate.DataType, manual DI registration, etc. ----
284+
for match in _TYPEOF_TYPE_RE.finditer(text):
285+
for target in _files_for(match.group(1)):
286+
if target != rel:
287+
edges.append(
288+
DynamicEdge(
289+
source=rel,
290+
target=target,
291+
edge_type="dynamic_uses",
292+
hint_source=f"{self.name}:typeof",
293+
)
294+
)
295+
276296
# ---- [assembly: InternalsVisibleTo("Other.Tests")] ----
277297
for match in _INTERNALS_VISIBLE_RE.finditer(text):
278298
friend = match.group(1)

packages/core/src/repowise/core/ingestion/dynamic_hints/xaml.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@
6868
re.IGNORECASE,
6969
)
7070

71+
# ``<prefix:TypeName ...>`` element-tag references. WPF / WinUI / MAUI
72+
# instantiate controls, converters and templates by writing them as
73+
# XAML elements; the consumer code never says ``using`` for those
74+
# types. We require the namespace prefix to avoid over-matching XAML's
75+
# built-in elements (``<Grid>``, ``<TextBlock>``…), and to keep noise
76+
# out we skip property-element syntax (``<Grid.Resources>``) by
77+
# rejecting tags whose name contains a dot.
78+
_ELEMENT_TAG_RE = re.compile(
79+
r"""<(\w+):([A-Z][\w]*)(?=[\s/>])""",
80+
)
81+
7182
# <ResourceDictionary Source="..."/> — both standalone and inside a
7283
# <ResourceDictionary.MergedDictionaries> block. Match attribute order
7384
# agnostically; only the Source value is needed.
@@ -200,6 +211,14 @@ def _extract_type_references(text: str, prefix_to_ns: dict[str, str]) -> set[str
200211
type_name = match.group(2)
201212
if type_name and type_name[0].isupper():
202213
names.add(type_name)
214+
# Element-tag references like ``<converters:BoolToVisibilityConverter ...>``.
215+
# The leading-uppercase guard is in the regex; we additionally skip
216+
# the ``ResourceDictionary`` tag because that's handled separately
217+
# and binding it to the C# type map would just produce noise.
218+
for match in _ELEMENT_TAG_RE.finditer(text):
219+
type_name = match.group(2)
220+
if type_name and type_name != "ResourceDictionary":
221+
names.add(type_name)
203222
# The xmlns prefixes themselves never name a type, but their target
204223
# namespaces are a useful signal — left here as a hook for future
205224
# enhancement that resolves "every type in that namespace" when no

packages/core/src/repowise/core/ingestion/parser.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,18 @@ def _extract_symbols(
670670
# Parent class detection
671671
parent_name = self._find_parent(def_node, config, receiver_nodes, src)
672672

673+
# C/C++ qualified definitions: ``void Foo::method() { … }``
674+
# carries the class as the scope of a ``qualified_identifier``
675+
# parent of the name node. Without this resolution, every
676+
# ``Class::method`` lands as a free function and bloats the
677+
# unused_export pass with thousands of method symbols.
678+
if (
679+
parent_name is None
680+
and file_info.language in ("cpp", "c")
681+
and name_nodes
682+
):
683+
parent_name = _qualified_cpp_parent(name_nodes[0], src)
684+
673685
# Upgrade function → method when a parent class is detected
674686
if parent_name and kind == "function":
675687
kind = "method"
@@ -990,6 +1002,33 @@ def _is_async_node(node: Node, src: str) -> bool:
9901002
return node.type == "async_function_definition" or any(c.type == "async" for c in node.children)
9911003

9921004

1005+
def _qualified_cpp_parent(name_node: Node, src: str) -> str | None:
1006+
"""Return the parent class for a C/C++ ``Class::method`` definition.
1007+
1008+
The captured ``@symbol.name`` for a qualified function definition
1009+
is the bare ``method`` identifier whose parent is a
1010+
``qualified_identifier`` carrying the class / namespace as its
1011+
``scope`` field. For multi-level qualifications (``NS::Foo::method``)
1012+
the relevant parent is still the innermost qualifier — namespaces
1013+
above it are not the symbol's containing type. Tree-sitter-cpp
1014+
represents this by nesting ``qualified_identifier`` left-recursively,
1015+
so the immediate parent's ``scope`` is always the right answer.
1016+
1017+
Returns ``None`` when the name node is not inside a qualified
1018+
identifier (i.e. plain free function).
1019+
"""
1020+
parent = name_node.parent
1021+
if parent is None or parent.type != "qualified_identifier":
1022+
return None
1023+
scope = parent.child_by_field_name("scope")
1024+
if scope is None:
1025+
return None
1026+
text = src[scope.start_byte : scope.end_byte].strip()
1027+
# ``scope`` may itself be a qualified path (``NS::Foo``); take the
1028+
# last component — that's the immediate enclosing type.
1029+
return text.rsplit("::", 1)[-1] or None
1030+
1031+
9931032
def _build_qualified_name(file_path: str, parent_name: str | None, name: str) -> str:
9941033
module = Path(file_path).with_suffix("").as_posix().replace("/", ".")
9951034
if parent_name:

packages/core/src/repowise/core/ingestion/queries/cpp.scm

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,22 @@
3636
)
3737
) @symbol.def
3838

39+
; Two-level qualified function: ReturnType NS::ClassName::method(params) { }
40+
; The grammar nests the qualified_identifier left-recursively, so we
41+
; need a separate pattern for each depth. Parser walks the captured
42+
; name's qualified-identifier parent to extract the class name, so the
43+
; deeper namespace prefix doesn't need to be captured here.
44+
(function_definition
45+
declarator: (function_declarator
46+
declarator: (qualified_identifier
47+
name: (qualified_identifier
48+
name: (identifier) @symbol.name
49+
)
50+
)
51+
parameters: (parameter_list) @symbol.params
52+
)
53+
) @symbol.def
54+
3955
; Class
4056
(class_specifier
4157
name: (type_identifier) @symbol.name

tests/unit/ingestion/test_cpp_extractors.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,24 @@ def test_struct_method_public_by_default(self, parser: ASTParser) -> None:
103103
assert by_name["Visible"].visibility == "public"
104104
assert by_name["Hidden"].visibility == "private"
105105

106+
def test_qualified_definition_binds_to_class(self, parser: ASTParser) -> None:
107+
"""``void Foo::method() { … }`` extracts as a method with parent=Foo."""
108+
src = b"""\
109+
void Foo::DoOne(int x) { }
110+
void NS::Bar::DoTwo(int y) { }
111+
void Free() { }
112+
"""
113+
result = parser.parse_file(_file(), src)
114+
by_name = {s.name: s for s in result.symbols}
115+
assert by_name["DoOne"].kind == "method"
116+
assert by_name["DoOne"].parent_name == "Foo"
117+
assert by_name["DoTwo"].kind == "method"
118+
# Two-level NS::Bar::DoTwo → parent is the innermost qualifier, Bar.
119+
assert by_name["DoTwo"].parent_name == "Bar"
120+
# Free function unaffected.
121+
assert by_name["Free"].kind == "function"
122+
assert by_name["Free"].parent_name is None
123+
106124
def test_dllexport_marks_exported_symbol(self, parser: ASTParser) -> None:
107125
src = b"""\
108126
extern "C" __declspec(dllexport) HRESULT DllRegisterServer(void) { return 0; }

tests/unit/ingestion/test_csharp_framework_edges.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,40 @@ def test_nameof_type_emits_dynamic_uses(self, tmp_path: Path) -> None:
306306
for e in edges
307307
)
308308

309+
def test_typeof_type_emits_dynamic_uses(self, tmp_path: Path) -> None:
310+
"""``typeof(MyConverter)`` references must surface as dynamic edges.
311+
312+
Critical for ``[JsonConverter(typeof(X))]``,
313+
``[TypeConverter(typeof(X))]``, ``DataTemplate.DataType =
314+
typeof(X)`` and manual DI registration.
315+
"""
316+
(tmp_path / "BoolConverter.cs").write_text(
317+
"namespace Acme;\npublic class BoolConverter { }\n"
318+
)
319+
(tmp_path / "Settings.cs").write_text(
320+
"""namespace Acme;
321+
public class Settings {
322+
[JsonConverter(typeof(BoolConverter))]
323+
public bool Flag { get; set; }
324+
}
325+
"""
326+
)
327+
edges = DotNetDynamicHints().extract(tmp_path)
328+
assert any(
329+
e.source == "Settings.cs"
330+
and e.target == "BoolConverter.cs"
331+
and e.hint_source.endswith(":typeof")
332+
for e in edges
333+
)
334+
335+
def test_typeof_lowercase_local_ignored(self, tmp_path: Path) -> None:
336+
"""``typeof(int)`` and lowercase identifiers must not bind."""
337+
(tmp_path / "Foo.cs").write_text(
338+
"namespace Acme;\npublic class Foo { void Go() { var t = typeof(int); } }\n"
339+
)
340+
edges = DotNetDynamicHints().extract(tmp_path)
341+
assert not any(e.hint_source.endswith(":typeof") for e in edges)
342+
309343
def test_nameof_lowercase_member_ignored(self, tmp_path: Path) -> None:
310344
"""``nameof(someLocal)`` must NOT bind — only type-looking
311345
identifiers are scanned to keep noise out.

tests/unit/ingestion/test_parser.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,9 +582,11 @@ def test_finds_class_in_header(self, parser: ASTParser) -> None:
582582
def test_finds_functions_in_source(self, parser: ASTParser) -> None:
583583
fi = _make_file_info("cpp_pkg/calculator.cpp", "cpp")
584584
result = parser.parse_file(fi, CPP_SOURCE)
585-
fns = [s for s in result.symbols if s.kind == "function"]
586-
# Qualified definitions like Calculator::add should be caught
587-
assert len(fns) >= 1
585+
# ``Calculator::add`` style qualified definitions now resolve to
586+
# ``kind=method`` with ``parent_name=Calculator``; free functions
587+
# stay ``kind=function``. Either way we expect symbols to land.
588+
callable_symbols = [s for s in result.symbols if s.kind in ("function", "method")]
589+
assert len(callable_symbols) >= 1
588590

589591
def test_parses_includes(self, parser: ASTParser) -> None:
590592
fi = _make_file_info("cpp_pkg/calculator.cpp", "cpp")

tests/unit/ingestion/test_xaml_dynamic_hints.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,25 @@ def test_xtype_markup_extension(self) -> None:
5959
refs = _extract_type_references(text, {"vm": "Acme.ViewModels"})
6060
assert "SettingsViewModel" in refs
6161

62+
def test_element_tag_with_prefix_extracted(self) -> None:
63+
"""``<converters:BoolToVisibilityConverter />`` registers ``BoolToVisibilityConverter``."""
64+
text = '<Page><converters:BoolToVisibilityConverter x:Key="b2v"/></Page>'
65+
refs = _extract_type_references(text, {"converters": "Acme.Converters"})
66+
assert "BoolToVisibilityConverter" in refs
67+
68+
def test_element_tag_property_syntax_skipped(self) -> None:
69+
"""``<Grid.Resources>`` is property-element syntax, not a type reference."""
70+
text = '<Grid><Grid.Resources/></Grid>'
71+
refs = _extract_type_references(text, {})
72+
assert "Resources" not in refs
73+
74+
def test_bare_xaml_element_not_a_type_reference(self) -> None:
75+
"""Built-in XAML elements (``<Grid>``, ``<TextBlock>``) must not bind."""
76+
text = '<Page><Grid><TextBlock/></Grid></Page>'
77+
refs = _extract_type_references(text, {})
78+
assert "Grid" not in refs
79+
assert "TextBlock" not in refs
80+
6281
def test_lowercase_attribute_value_skipped(self) -> None:
6382
# Attribute values that don't start with an uppercase letter
6483
# (e.g. property names accidentally captured) are skipped.

0 commit comments

Comments
 (0)