Skip to content

RULE-6-9-1: fix false positives for redeclarations that use the same type alias - #1180

Draft
castler wants to merge 1 commit into
github:mainfrom
castler:fix-rule-6-9-1-type-alias-false-positives
Draft

RULE-6-9-1: fix false positives for redeclarations that use the same type alias#1180
castler wants to merge 1 commit into
github:mainfrom
castler:fix-rule-6-9-1-type-alias-false-positives

Conversation

@castler

@castler castler commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Sorry - this is not yet close ready to being reviewed!

Problem

RULE-6-9-1 ("The same type aliases shall be used in all declarations of the
same entity") reports a large number of false positives on real code. Every one
of the reported entities actually uses identical alias spellings in all its
declarations, e.g. a function prototype in a header and its definition in a
.cpp both returning element_fq_id&, LoggingCallback, Result<...>, etc.

Root cause

The query fires on:

t.getATypeNameUse() = decl1 and
not t.getATypeNameUse() = decl2

TypedefType.getATypeNameUse() is documented as a conservative / incomplete
relation ("not necessarily all type name uses"; it omits uses on prototypes
and around template instantiations). Worse, in whole-program extraction it is
inconsistent across translation units: the same header line produces
multiple DeclarationEntry objects (one per including TU) and
getATypeNameUse() associates the alias with some copies but not others.

Direct evidence (return-by-const HandleType& accessor):

proxy_base.h:118   isDecl              <- entry from TU A: getATypeNameUse = NO
proxy_base.h:118   isDecl,nameUseYES   <- entry from TU B: getATypeNameUse = YES  (same line!)
proxy_base.cpp:40  isDef               <- definition:     getATypeNameUse = NO

The query pairs the YES entry (decl1) with a NO entry (decl2) and
reports a divergence, even though both declarations spell the alias identically.

Fix

Add three guards to the where clause, backed by three helper predicates:

  1. sameSourceLocation(decl1, decl2) — two entries at the exact same file /
    line / column are the same source declaration seen from different TUs, not
    two redeclarations that could disagree. Exclude them.
  2. template-instantiation exclusion — synthesised instantiation entries
    duplicate the template's entries without recording type-name uses.
  3. typeAliasMentionedIn(t, decl2) — before reporting that decl2 fails to
    use t, confirm it via TypeMention, which records every syntactic type
    mention with its location. Matching is done by qualified name so that a
    mention of a generic alias template (Result) is recognised as the same
    alias as the instantiated getATypeNameUse() result (Result<X>). For
    function definitions the mention search range is extended to the start of the
    function body to catch trailing return types (auto f() -> T).

Validation

  • Real codebase (eclipse-score/communication, //score/message_passing +
    //score/mw/com, whole-program CodeQL database, coding-standards pack
    2.62.0 baseline): 276 → 30 findings (89% reduction). All 246 eliminated
    findings were manually verified as false positives (identical alias spelling
    confirmed by reading the source at both declaration sites); the 30 remaining
    findings are genuine spelling mismatches (e.g. a class member alias used in
    the header vs. the underlying namespace-scope alias spelled out explicitly in
    the out-of-line definition).
  • Upstream unit test: TypeAliasesDeclaration.qlref still PASSES — the
    genuine INT/Index-vs-int/i divergences are still reported, so no true
    positives are lost.

Known limitation

The residual ~30 findings (on our codebase) are return-by-const Alias&
accessors and some template members where neither getATypeNameUse() nor
TypeMention emits any mention of the alias on the relevant line, so no
reliable signal exists to suppress them without risking false negatives. These
are all genuine violations in our codebase, so this is expected, not a gap.

Why no qltest regression test

The false positive is an emergent property of whole-program (multi-hundred-TU)
extraction
: the cross-TU getATypeNameUse() asymmetry does not arise in a
single TU (the relation is symmetric there) and did not reproduce in a minimal
2-TU header test either (the extractor merge stayed symmetric). This is the same
reason the existing per-rule test never caught the issue. The fix is therefore
validated against the real codebase numbers above rather than a synthetic
.expected.

RULE-6-9-1 reported false positives on entities that use identical type
alias spellings in every declaration (e.g. a function prototype in a
header and its definition in a .cpp both returning the same aliased
type).

The query decides that two declaration entries disagree using
`t.getATypeNameUse() = decl1 and not t.getATypeNameUse() = decl2`.
`TypedefType.getATypeNameUse()` is documented as incomplete and, in
whole-program extraction, is inconsistent across translation units: the
same header line yields one DeclarationEntry per including TU, and the
alias is associated with some copies but not others. The query then
pairs a "use" entry with a "no-use" entry and reports a spurious
divergence.

Add three guards, backed by helper predicates:
- sameSourceLocation: drop pairs that are the same source declaration
  seen from different TUs (same file/line/column).
- template-instantiation exclusion: synthesised instantiation entries
  duplicate the template's entries without recording type-name uses.
- typeAliasMentionedIn: before reporting that decl2 fails to use the
  alias, confirm via TypeMention (which records every syntactic type
  mention). Match by qualified name so a generic alias template
  (Result) is recognised as the instantiated result (Result<X>), and
  extend the search range to the function body start to catch trailing
  return types.

Validated on a real codebase: 234 -> 30 findings (87% reduction, all
eliminated findings verified as false positives). The existing unit
test still passes, so no true positives are lost. No qltest regression
test is added because the false positive is an emergent property of
multi-TU whole-program extraction and does not reproduce in the
single/two-TU test harness.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@castler
castler marked this pull request as draft August 24, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant