-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[clang-tidy] readability-container-size-empty
: Correctly generating fix hints when size method is called from implicit this
#152486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ixit when `size` method's base is `this`
@llvm/pr-subscribers-clang-tools-extra Author: None (flovent) ChangesCorrectly generating fix hints when size method is inherited from parent class (When Closes #152387. Full diff: https://github.com/llvm/llvm-project/pull/152486.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
index ce736a8d16023..c4dc319f23c38 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -238,9 +238,12 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
? MemberCallObject
: (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
FixItHint Hint;
- std::string ReplacementText = std::string(
- Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
+ std::string ReplacementText =
+ E->isImplicitCXXThis()
+ ? ""
+ : std::string(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(E->getSourceRange()),
+ *Result.SourceManager, getLangOpts()));
const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E);
if (isBinaryOrTernary(E) || isa<UnaryOperator>(E) ||
(OpCallExpr && (OpCallExpr->getOperator() == OO_Star))) {
@@ -251,6 +254,8 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
// This can happen if the object is a smart pointer. Don't add anything
// because a '->' is already there (PR#51776), just call the method.
ReplacementText += "empty()";
+ } else if (E->isImplicitCXXThis()) {
+ ReplacementText += "empty()";
} else if (E->getType()->isPointerType())
ReplacementText += "->empty()";
else
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2dc5c73073cf8..a0c03754e8fbf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,10 @@ Changes in existing checks
<clang-tidy/checks/portability/template-virtual-member-function>` check to
avoid false positives on pure virtual member functions.
+- Improved :doc:`readability-container-size-empty
+ <clang-tidy/checks/portability/container-size-empty>` check by correctly
+ generating fix hints when size method is inherited from parent class.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check by ignoring
declarations in system headers.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
index 2fd0b2224cb1c..462db5acbaade 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -895,3 +895,16 @@ namespace PR94454 {
int operator""_ci() { return 0; }
auto eq = 0_ci == 0;
}
+
+namespace PR152387 {
+
+class foo : public std::string{
+ void doit() {
+ if (!size()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: if (empty()) {
+ }
+ }
+};
+
+}
|
@llvm/pr-subscribers-clang-tidy Author: None (flovent) ChangesCorrectly generating fix hints when size method is inherited from parent class (When Closes #152387. Full diff: https://github.com/llvm/llvm-project/pull/152486.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
index ce736a8d16023..c4dc319f23c38 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -238,9 +238,12 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
? MemberCallObject
: (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
FixItHint Hint;
- std::string ReplacementText = std::string(
- Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
+ std::string ReplacementText =
+ E->isImplicitCXXThis()
+ ? ""
+ : std::string(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(E->getSourceRange()),
+ *Result.SourceManager, getLangOpts()));
const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E);
if (isBinaryOrTernary(E) || isa<UnaryOperator>(E) ||
(OpCallExpr && (OpCallExpr->getOperator() == OO_Star))) {
@@ -251,6 +254,8 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
// This can happen if the object is a smart pointer. Don't add anything
// because a '->' is already there (PR#51776), just call the method.
ReplacementText += "empty()";
+ } else if (E->isImplicitCXXThis()) {
+ ReplacementText += "empty()";
} else if (E->getType()->isPointerType())
ReplacementText += "->empty()";
else
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2dc5c73073cf8..a0c03754e8fbf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,10 @@ Changes in existing checks
<clang-tidy/checks/portability/template-virtual-member-function>` check to
avoid false positives on pure virtual member functions.
+- Improved :doc:`readability-container-size-empty
+ <clang-tidy/checks/portability/container-size-empty>` check by correctly
+ generating fix hints when size method is inherited from parent class.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check by ignoring
declarations in system headers.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
index 2fd0b2224cb1c..462db5acbaade 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -895,3 +895,16 @@ namespace PR94454 {
int operator""_ci() { return 0; }
auto eq = 0_ci == 0;
}
+
+namespace PR152387 {
+
+class foo : public std::string{
+ void doit() {
+ if (!size()) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: if (empty()) {
+ }
+ }
+};
+
+}
|
readability-container-size-empty
: Correctly generating fix hints when size method is inherited from parent classreadability-container-size-empty
: Correctly generating fix hints when size method is called from implicit this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Baranov Victor <[email protected]>
Co-authored-by: Baranov Victor <[email protected]>
Co-authored-by: EugeneZelenko <[email protected]>
Correctly generating fix hints when size method is called from implicit this.
Closes #152387.