This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] check for using declaration qualification
Needs RevisionPublic

Authored by Naysh on Dec 5 2018, 4:39 PM.

Details

Summary

This patch adds a "Qualified Aliases" check, based off the guidelines in https://abseil.io/tips/119, to the abseil module.

The check flags any using declarations which are not fully qualified.

Diff Detail

Event Timeline

Naysh created this revision.Dec 5 2018, 4:39 PM
Eugene.Zelenko added a project: Restricted Project.
Eugene.Zelenko added inline comments.
clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp
31

Please remove unnecessary empty line.

33

Please remove unnecessary empty line.

34

const auto *

clang-tidy/abseil/QualifiedAliasesCheck.cpp
13

Please remove unnecessary empty line.

clang-tidy/abseil/SafelyScopedCheck.cpp
27

Please run Clang-format over patch.

docs/ReleaseNotes.rst
73

Please enclose using in ``. Same in other places, including documentation.

docs/clang-tidy/checks/abseil-alias-free-headers.rst
10

Are there guidelines?

docs/clang-tidy/checks/abseil-qualified-aliases.rst
24

Please remove unnecessary empty line.

docs/clang-tidy/checks/abseil-safely-scoped.rst
28

Please remove unnecessary empty line.

test/clang-tidy/abseil-qualified-aliases.cpp
27

Please remove unnecessary empty line.

test/clang-tidy/abseil-safely-scoped.cpp
27

Please remove unnecessary empty line.

Eugene.Zelenko added inline comments.Dec 5 2018, 7:22 PM
docs/clang-tidy/checks/abseil-anonymous-enclosed-aliases.rst
20

Are there guidelines?

docs/clang-tidy/checks/abseil-qualified-aliases.rst
22

Please make text consistent across all these checks.

alexfh requested changes to this revision.Dec 6 2018, 6:00 AM

Could you send each check in a separate patch, please? This makes the review much more straightforward and I'm sure it will be faster too.

This revision now requires changes to proceed.Dec 6 2018, 6:00 AM
alexfh added inline comments.Dec 6 2018, 6:01 AM
clang-tidy/abseil/AbseilTidyModule.cpp
25

Please upload the patch with full context. See https://llvm.org/docs/Phabricator.html

Naysh updated this revision to Diff 177116.EditedDec 6 2018, 9:20 PM
Naysh retitled this revision from [clang-tidy] check for using declaration scope and qualification to [clang-tidy] check for using declaration qualification.
Naysh edited the summary of this revision. (Show Details)

To respond to alexfh's request to split this submission into four separate patches (corresponding to each of the introduced checks), I've updated the patch to only include the "Qualified Aliases" check.

I'll add the other checks as separate patches.

Naysh edited the summary of this revision. (Show Details)Dec 6 2018, 9:32 PM
aaron.ballman added inline comments.Dec 10 2018, 4:47 AM
clang-tidy/abseil/QualifiedAliasesCheck.cpp
30

Drop the top-level const qualifier (here and elsewhere).

35

No need for the assignment.

40

This is missing a check for whether the referenced name is within the current namespace.

44–45

Do not put HTML links into the diagnostic -- the wording should stand on its own. I would probably phrase it "using declaration should use a fully qualified name" or something along those lines.

clang-tidy/abseil/QualifiedAliasesCheck.h
20

This should be fixed.

test/clang-tidy/abseil-qualified-aliases.cpp
23–24

I don't think this should warn, per the tip.

Naysh marked 4 inline comments as done.Dec 10 2018, 5:25 PM
JonasToth resigned from this revision.Dec 30 2019, 7:59 AM
alexfh requested changes to this revision.Oct 12 2020, 6:57 AM
This revision now requires changes to proceed.Oct 12 2020, 6:57 AM
test/clang-tidy/abseil-safely-scoped.cpp