From 3b45888622857cd5e3bfd1fbaa2de32421f70eec Mon Sep 17 00:00:00 2001 From: Myriad-Dreamin <35292584+Myriad-Dreamin@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:03:18 +0800 Subject: [PATCH] fix(semantic-tokens): filter ineligible highlight references (#434) Add a reusable declaration-name eligibility helper that mirrors clangd's `canHighlightName`, use it to suppress unsupported reference tokens in the semantic-token collector, and cover the change with focused semantic-token regression tests plus a constructor/destructor positive case. ## Summary by CodeRabbit * **Bug Fixes** * Improved semantic token highlighting to suppress ineligible operator references lacking meaningful source text. * Ensured constructor and destructor names remain properly highlighted with correct visual modifiers. * **Tests** * Added test coverage for semantic token highlighting behavior across various declaration types. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/clice-io/clice/pull/434?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) --- .gitignore | 3 +- src/feature/semantic_tokens.cpp | 32 +++++++++++++++ tests/unit/feature/semantic_tokens_tests.cpp | 42 ++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index abd5c2a6..cf7a020f 100644 --- a/.gitignore +++ b/.gitignore @@ -68,8 +68,7 @@ tests/unit/Local/ .pixi/* !.pixi/config.toml -.codex/ +.codex .claude/* !.claude/CLAUDE.md !.claude/commands/ -openspec/ diff --git a/src/feature/semantic_tokens.cpp b/src/feature/semantic_tokens.cpp index 13cf0674..9c4db1e0 100644 --- a/src/feature/semantic_tokens.cpp +++ b/src/feature/semantic_tokens.cpp @@ -34,6 +34,34 @@ bool is_dependent(const clang::Decl* D) { return isa(D); } +/// Whether a declaration name is backed by source text that should be highlighted. +bool can_highlight_name(clang::DeclarationName name) { + switch(name.getNameKind()) { + case clang::DeclarationName::Identifier: { + auto* info = name.getAsIdentifierInfo(); + return info && !info->getName().empty(); + } + + case clang::DeclarationName::CXXConstructorName: + case clang::DeclarationName::CXXDestructorName: { + return true; + } + + case clang::DeclarationName::CXXConversionFunctionName: + case clang::DeclarationName::CXXOperatorName: + case clang::DeclarationName::CXXDeductionGuideName: + case clang::DeclarationName::CXXLiteralOperatorName: + case clang::DeclarationName::CXXUsingDirective: + case clang::DeclarationName::ObjCZeroArgSelector: + case clang::DeclarationName::ObjCOneArgSelector: + case clang::DeclarationName::ObjCMultiArgSelector: { + return false; + } + } + + std::unreachable(); +} + /// Returns true if `decl` is considered to be from a default/system library. /// This currently checks the systemness of the file by include type, although /// different heuristics may be used in the future (e.g. sysroot paths). @@ -171,6 +199,10 @@ public: void handleDeclOccurrence(const clang::NamedDecl* decl, RelationKind relation, clang::SourceLocation location) { + if(relation.isReference() && !can_highlight_name(decl->getDeclName())) { + return; + } + std::uint32_t modifiers = 0; if(relation.is_one_of(RelationKind::Definition)) { // todo: clangd add both Declaration and Definition modifiers for definitions. diff --git a/tests/unit/feature/semantic_tokens_tests.cpp b/tests/unit/feature/semantic_tokens_tests.cpp index 71e922b1..e5e61399 100644 --- a/tests/unit/feature/semantic_tokens_tests.cpp +++ b/tests/unit/feature/semantic_tokens_tests.cpp @@ -140,6 +140,10 @@ void EXPECT_TOKEN(llvm::StringRef name, ASSERT_EQ(token->modifiers, expected_modifiers); } +void EXPECT_NO_TOKEN(llvm::StringRef name) { + ASSERT_TRUE(find_by_range(name) == nullptr); +} + TEST_CASE(BasicLexicalKinds) { run_utf8(R"cpp( @d1[#define] @m0[FOO] @@ -266,6 +270,44 @@ int main() { EXPECT_TOKEN("x3", SymbolKind::Variable, 0); } +TEST_CASE(IneligibleOperatorReferenceIsSuppressed) { + run_utf8(R"cpp( +struct S {}; + +S operator+(S lhs, S rhs); + +void use(S lhs, S rhs) { + (void)(lhs @plus[+] rhs); +} +)cpp"); + + EXPECT_NO_TOKEN("plus"); +} + +TEST_CASE(ConstructorAndDestructorNamesRemainHighlighted) { + run_utf8(R"cpp( +struct S { + @ctor_decl[S](); + @dtor_decl[~]S(); +}; + +S::@ctor_def[S]() {} + +void use(S* value) { + value->@dtor_ref[~]S(); +} +)cpp"); + + auto declaration = modifier_mask({SymbolModifiers::Declaration}); + auto definition = modifier_mask({SymbolModifiers::Definition}); + auto special_member = modifier_mask({SymbolModifiers::ConstructorOrDestructor}); + + EXPECT_TOKEN("ctor_decl", SymbolKind::Method, declaration | special_member); + EXPECT_TOKEN("dtor_decl", SymbolKind::Method, declaration | special_member); + EXPECT_TOKEN("ctor_def", SymbolKind::Method, definition | special_member); + EXPECT_TOKEN("dtor_ref", SymbolKind::Method, special_member); +} + TEST_CASE(LegacyVarDeclTemplates) { run_utf8(R"cpp( extern int @x1[x];