From c87d2a613ec0e99e43ea683743736c8acc9bd47c Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Mon, 3 Oct 2016 08:11:50 +0000 Subject: [PATCH] [analyzer] Improve CloneChecker diagnostics Highlight code clones referenced by the warning message with the help of the extra notes feature recently introduced in r283092. Change warning text to more clang-ish. Remove suggestions from the copy-paste error checker diagnostics, because currently our suggestions are strictly 50% wrong (we do not know which of the two code clones contains the error), and for that reason we should not sound as if we're actually suggesting this. Hopefully a better solution would bring them back. Make sure the suspicious clone pair structure always mentions the correct variable for the second clone. Differential Revision: https://reviews.llvm.org/D24916 llvm-svn: 283094 --- clang/include/clang/Analysis/CloneDetection.h | 10 +- clang/lib/Analysis/CloneDetection.cpp | 24 +++-- .../StaticAnalyzer/Checkers/CloneChecker.cpp | 102 +++++++++--------- clang/test/Analysis/copypaste/blocks.cpp | 4 +- .../Analysis/copypaste/function-try-block.cpp | 4 +- clang/test/Analysis/copypaste/functions.cpp | 4 +- .../Analysis/copypaste/macro-complexity.cpp | 8 +- clang/test/Analysis/copypaste/macros.cpp | 10 +- clang/test/Analysis/copypaste/objc-methods.m | 4 +- .../plist-diagnostics-notes-as-events.cpp | 97 +++++++++++++++++ .../Analysis/copypaste/plist-diagnostics.cpp | 71 ++++++++++++ .../test/Analysis/copypaste/sub-sequences.cpp | 4 +- .../Analysis/copypaste/suspicious-clones.cpp | 20 ++-- .../Analysis/copypaste/text-diagnostics.cpp | 17 +++ 14 files changed, 289 insertions(+), 90 deletions(-) create mode 100644 clang/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp create mode 100644 clang/test/Analysis/copypaste/plist-diagnostics.cpp create mode 100644 clang/test/Analysis/copypaste/text-diagnostics.cpp diff --git a/clang/include/clang/Analysis/CloneDetection.h b/clang/include/clang/Analysis/CloneDetection.h index 693e4a0bfad4..51cad7a96d6f 100644 --- a/clang/include/clang/Analysis/CloneDetection.h +++ b/clang/include/clang/Analysis/CloneDetection.h @@ -128,6 +128,10 @@ public: /// This method should only be called on a non-empty StmtSequence object. SourceLocation getEndLoc() const; + /// Returns the source range of the whole sequence - from the beginning + /// of the first statement to the end of the last statement. + SourceRange getSourceRange() const; + bool operator==(const StmtSequence &Other) const { return std::tie(S, StartIndex, EndIndex) == std::tie(Other.S, Other.StartIndex, Other.EndIndex); @@ -250,14 +254,14 @@ public: /// The variable which referencing in this clone was against the pattern. const VarDecl *Variable; /// Where the variable was referenced. - SourceRange VarRange; + const Stmt *Mention; /// The variable that should have been referenced to follow the pattern. /// If Suggestion is a nullptr then it's not possible to fix the pattern /// by referencing a different variable in this clone. const VarDecl *Suggestion; - SuspiciousCloneInfo(const VarDecl *Variable, SourceRange Range, + SuspiciousCloneInfo(const VarDecl *Variable, const Stmt *Mention, const VarDecl *Suggestion) - : Variable(Variable), VarRange(Range), Suggestion(Suggestion) {} + : Variable(Variable), Mention(Mention), Suggestion(Suggestion) {} SuspiciousCloneInfo() {} }; /// The first clone in the pair which always has a suggested variable. diff --git a/clang/lib/Analysis/CloneDetection.cpp b/clang/lib/Analysis/CloneDetection.cpp index a91ccaa4d2cd..dba89eca1f39 100644 --- a/clang/lib/Analysis/CloneDetection.cpp +++ b/clang/lib/Analysis/CloneDetection.cpp @@ -82,6 +82,10 @@ SourceLocation StmtSequence::getStartLoc() const { SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); } +SourceRange StmtSequence::getSourceRange() const { + return SourceRange(getStartLoc(), getEndLoc()); +} + namespace { /// \brief Analyzes the pattern of the referenced variables in a statement. @@ -91,11 +95,11 @@ class VariablePattern { struct VariableOccurence { /// The index of the associated VarDecl in the Variables vector. size_t KindID; - /// The source range in the code where the variable was referenced. - SourceRange Range; + /// The statement in the code where the variable was referenced. + const Stmt *Mention; - VariableOccurence(size_t KindID, SourceRange Range) - : KindID(KindID), Range(Range) {} + VariableOccurence(size_t KindID, const Stmt *Mention) + : KindID(KindID), Mention(Mention) {} }; /// All occurences of referenced variables in the order of appearance. @@ -107,19 +111,19 @@ class VariablePattern { /// \brief Adds a new variable referenced to this pattern. /// \param VarDecl The declaration of the variable that is referenced. /// \param Range The SourceRange where this variable is referenced. - void addVariableOccurence(const VarDecl *VarDecl, SourceRange Range) { + void addVariableOccurence(const VarDecl *VarDecl, const Stmt *Mention) { // First check if we already reference this variable for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) { if (Variables[KindIndex] == VarDecl) { // If yes, add a new occurence that points to the existing entry in // the Variables vector. - Occurences.emplace_back(KindIndex, Range); + Occurences.emplace_back(KindIndex, Mention); return; } } // If this variable wasn't already referenced, add it to the list of // referenced variables and add a occurence that points to this new entry. - Occurences.emplace_back(Variables.size(), Range); + Occurences.emplace_back(Variables.size(), Mention); Variables.push_back(VarDecl); } @@ -134,7 +138,7 @@ class VariablePattern { // Check if S is a reference to a variable. If yes, add it to the pattern. if (auto D = dyn_cast(S)) { if (auto VD = dyn_cast(D->getDecl()->getCanonicalDecl())) - addVariableOccurence(VD, D->getSourceRange()); + addVariableOccurence(VD, D); } // Recursively check all children of the given statement. @@ -208,7 +212,7 @@ public: // Store information about the first clone. FirstMismatch->FirstCloneInfo = CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo( - Variables[ThisOccurence.KindID], ThisOccurence.Range, + Variables[ThisOccurence.KindID], ThisOccurence.Mention, FirstSuggestion); // Same as above but with the other clone. We do this for both clones as @@ -221,7 +225,7 @@ public: // Store information about the second clone. FirstMismatch->SecondCloneInfo = CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo( - Variables[ThisOccurence.KindID], OtherOccurence.Range, + Other.Variables[OtherOccurence.KindID], OtherOccurence.Mention, SecondSuggestion); // SuspiciousClonePair guarantees that the first clone always has a diff --git a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp index b4ac223c0224..6fa5732d10cb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp @@ -16,8 +16,10 @@ #include "ClangSACheckers.h" #include "clang/Analysis/CloneDetection.h" #include "clang/Basic/Diagnostic.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" using namespace clang; @@ -27,6 +29,7 @@ namespace { class CloneChecker : public Checker { mutable CloneDetector Detector; + mutable std::unique_ptr BT_Exact, BT_Suspicious; public: void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, @@ -36,12 +39,12 @@ public: AnalysisManager &Mgr, BugReporter &BR) const; /// \brief Reports all clones to the user. - void reportClones(SourceManager &SM, AnalysisManager &Mgr, + void reportClones(BugReporter &BR, AnalysisManager &Mgr, int MinComplexity) const; /// \brief Reports only suspicious clones to the user along with informaton /// that explain why they are suspicious. - void reportSuspiciousClones(SourceManager &SM, AnalysisManager &Mgr, + void reportSuspiciousClones(BugReporter &BR, AnalysisManager &Mgr, int MinComplexity) const; }; } // end anonymous namespace @@ -70,79 +73,82 @@ void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU, "ReportNormalClones", true, this); if (ReportSuspiciousClones) - reportSuspiciousClones(BR.getSourceManager(), Mgr, MinComplexity); + reportSuspiciousClones(BR, Mgr, MinComplexity); if (ReportNormalClones) - reportClones(BR.getSourceManager(), Mgr, MinComplexity); + reportClones(BR, Mgr, MinComplexity); } -void CloneChecker::reportClones(SourceManager &SM, AnalysisManager &Mgr, +static PathDiagnosticLocation makeLocation(const StmtSequence &S, + AnalysisManager &Mgr) { + ASTContext &ACtx = Mgr.getASTContext(); + return PathDiagnosticLocation::createBegin( + S.front(), ACtx.getSourceManager(), + Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl())); +} + +void CloneChecker::reportClones(BugReporter &BR, AnalysisManager &Mgr, int MinComplexity) const { std::vector CloneGroups; Detector.findClones(CloneGroups, MinComplexity); - DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic(); - - unsigned WarnID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Warning, - "Detected code clone."); - - unsigned NoteID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Note, - "Related code clone is here."); + if (!BT_Exact) + BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone")); for (CloneDetector::CloneGroup &Group : CloneGroups) { // We group the clones by printing the first as a warning and all others // as a note. - DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID); - for (unsigned i = 1; i < Group.Sequences.size(); ++i) { - DiagEngine.Report(Group.Sequences[i].getStartLoc(), NoteID); - } + auto R = llvm::make_unique( + *BT_Exact, "Duplicate code detected", + makeLocation(Group.Sequences.front(), Mgr)); + R->addRange(Group.Sequences.front().getSourceRange()); + + for (unsigned i = 1; i < Group.Sequences.size(); ++i) + R->addNote("Similar code here", + makeLocation(Group.Sequences[i], Mgr), + Group.Sequences[i].getSourceRange()); + BR.emitReport(std::move(R)); } } -void CloneChecker::reportSuspiciousClones(SourceManager &SM, +void CloneChecker::reportSuspiciousClones(BugReporter &BR, AnalysisManager &Mgr, int MinComplexity) const { std::vector Clones; Detector.findSuspiciousClones(Clones, MinComplexity); - DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic(); + if (!BT_Suspicious) + BT_Suspicious.reset( + new BugType(this, "Suspicious code clone", "Code clone")); - auto SuspiciousCloneWarning = DiagEngine.getCustomDiagID( - DiagnosticsEngine::Warning, "suspicious code clone detected; did you " - "mean to use %0?"); - - auto RelatedCloneNote = DiagEngine.getCustomDiagID( - DiagnosticsEngine::Note, "suggestion is based on the usage of this " - "variable in a similar piece of code"); - - auto RelatedSuspiciousCloneNote = DiagEngine.getCustomDiagID( - DiagnosticsEngine::Note, "suggestion is based on the usage of this " - "variable in a similar piece of code; did you " - "mean to use %0?"); + ASTContext &ACtx = BR.getContext(); + SourceManager &SM = ACtx.getSourceManager(); + AnalysisDeclContext *ADC = + Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl()); for (CloneDetector::SuspiciousClonePair &Pair : Clones) { - // The first clone always has a suggestion and we report it to the user - // along with the place where the suggestion should be used. - DiagEngine.Report(Pair.FirstCloneInfo.VarRange.getBegin(), - SuspiciousCloneWarning) - << Pair.FirstCloneInfo.VarRange << Pair.FirstCloneInfo.Suggestion; + // FIXME: We are ignoring the suggestions currently, because they are + // only 50% accurate (even if the second suggestion is unavailable), + // which may confuse the user. + // Think how to perform more accurate suggestions? - // The second clone can have a suggestion and if there is one, we report - // that suggestion to the user. - if (Pair.SecondCloneInfo.Suggestion) { - DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(), - RelatedSuspiciousCloneNote) - << Pair.SecondCloneInfo.VarRange << Pair.SecondCloneInfo.Suggestion; - continue; - } + auto R = llvm::make_unique( + *BT_Suspicious, + "Potential copy-paste error; did you really mean to use '" + + Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?", + PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM, + ADC)); + R->addRange(Pair.FirstCloneInfo.Mention->getSourceRange()); - // If there isn't a suggestion in the second clone, we only inform the - // user where we got the idea that his code could contain an error. - DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(), - RelatedCloneNote) - << Pair.SecondCloneInfo.VarRange; + R->addNote("Similar code using '" + + Pair.SecondCloneInfo.Variable->getNameAsString() + "' here", + PathDiagnosticLocation::createBegin(Pair.SecondCloneInfo.Mention, + SM, ADC), + Pair.SecondCloneInfo.Mention->getSourceRange()); + + BR.emitReport(std::move(R)); } } diff --git a/clang/test/Analysis/copypaste/blocks.cpp b/clang/test/Analysis/copypaste/blocks.cpp index 0bd981267e8d..133b5cbcd6ac 100644 --- a/clang/test/Analysis/copypaste/blocks.cpp +++ b/clang/test/Analysis/copypaste/blocks.cpp @@ -4,14 +4,14 @@ void log(); -auto BlockA = ^(int a, int b){ // expected-warning{{Detected code clone.}} +auto BlockA = ^(int a, int b){ // expected-warning{{Duplicate code detected}} log(); if (a > b) return a; return b; }; -auto BlockB = ^(int a, int b){ // expected-note{{Related code clone is here.}} +auto BlockB = ^(int a, int b){ // expected-note{{Similar code here}} log(); if (a > b) return a; diff --git a/clang/test/Analysis/copypaste/function-try-block.cpp b/clang/test/Analysis/copypaste/function-try-block.cpp index b13096d949a9..7a69097579ab 100644 --- a/clang/test/Analysis/copypaste/function-try-block.cpp +++ b/clang/test/Analysis/copypaste/function-try-block.cpp @@ -3,7 +3,7 @@ // Tests if function try blocks are correctly handled. void nonCompoundStmt1(int& x) - try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Detected code clone.}} + try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Duplicate code detected}} void nonCompoundStmt2(int& x) - try { x += 1; } catch(...) { x -= 1; } // expected-note{{Related code clone is here.}} + try { x += 1; } catch(...) { x -= 1; } // expected-note{{Similar code here}} diff --git a/clang/test/Analysis/copypaste/functions.cpp b/clang/test/Analysis/copypaste/functions.cpp index 2a871f74b036..c95443de72d1 100644 --- a/clang/test/Analysis/copypaste/functions.cpp +++ b/clang/test/Analysis/copypaste/functions.cpp @@ -4,14 +4,14 @@ void log(); -int max(int a, int b) { // expected-warning{{Detected code clone.}} +int max(int a, int b) { // expected-warning{{Duplicate code detected}} log(); if (a > b) return a; return b; } -int maxClone(int x, int y) { // expected-note{{Related code clone is here.}} +int maxClone(int x, int y) { // expected-note{{Similar code here}} log(); if (x > y) return x; diff --git a/clang/test/Analysis/copypaste/macro-complexity.cpp b/clang/test/Analysis/copypaste/macro-complexity.cpp index 58b884ab4365..aca4df1d025c 100644 --- a/clang/test/Analysis/copypaste/macro-complexity.cpp +++ b/clang/test/Analysis/copypaste/macro-complexity.cpp @@ -11,11 +11,11 @@ // This confirms that with the current configuration the macro body would be // considered large enough to pass the MinimumCloneComplexity constraint. -int manualMacro(int a, int b) { // expected-warning{{Detected code clone.}} +int manualMacro(int a, int b) { // expected-warning{{Duplicate code detected}} return a > b ? -a * a : -b * b; } -int manualMacroClone(int a, int b) { // expected-note{{Related code clone is here.}} +int manualMacroClone(int a, int b) { // expected-note{{Similar code here}} return a > b ? -a * a : -b * b; } @@ -41,10 +41,10 @@ int macroClone(int a, int b) { #define NEG(A) -(A) -int nestedMacros() { // expected-warning{{Detected code clone.}} +int nestedMacros() { // expected-warning{{Duplicate code detected}} return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1)))))))))); } -int nestedMacrosClone() { // expected-note{{Related code clone is here.}} +int nestedMacrosClone() { // expected-note{{Similar code here}} return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1)))))))))); } diff --git a/clang/test/Analysis/copypaste/macros.cpp b/clang/test/Analysis/copypaste/macros.cpp index 170e034095d3..db9b4c6ee2e4 100644 --- a/clang/test/Analysis/copypaste/macros.cpp +++ b/clang/test/Analysis/copypaste/macros.cpp @@ -5,7 +5,7 @@ // to have the same complexity value. Macros have smaller complexity values // and need to be in their own hash group. -int foo(int a) { // expected-warning{{Detected code clone.}} +int foo(int a) { // expected-warning{{Duplicate code detected}} a = a + 1; a = a + 1 / 1; a = a + 1 + 1 + 1; @@ -15,7 +15,7 @@ int foo(int a) { // expected-warning{{Detected code clone.}} return a; } -int fooClone(int a) { // expected-note{{Related code clone is here.}} +int fooClone(int a) { // expected-note{{Similar code here}} a = a + 1; a = a + 1 / 1; a = a + 1 + 1 + 1; @@ -30,7 +30,7 @@ int fooClone(int a) { // expected-note{{Related code clone is here.}} #define ASSIGN(T, V) T = T + V -int macro(int a) { // expected-warning{{Detected code clone.}} +int macro(int a) { // expected-warning{{Duplicate code detected}} ASSIGN(a, 1); ASSIGN(a, 1 / 1); ASSIGN(a, 1 + 1 + 1); @@ -40,7 +40,7 @@ int macro(int a) { // expected-warning{{Detected code clone.}} return a; } -int macroClone(int a) { // expected-note{{Related code clone is here.}} +int macroClone(int a) { // expected-note{{Similar code here}} ASSIGN(a, 1); ASSIGN(a, 1 / 1); ASSIGN(a, 1 + 1 + 1); @@ -54,7 +54,7 @@ int macroClone(int a) { // expected-note{{Related code clone is here.}} #define EMPTY -int fooFalsePositiveClone(int a) { // expected-note{{Related code clone is here.}} +int fooFalsePositiveClone(int a) { // expected-note{{Similar code here}} a = EMPTY a + 1; a = a + 1 / 1; a = a + 1 + 1 + 1; diff --git a/clang/test/Analysis/copypaste/objc-methods.m b/clang/test/Analysis/copypaste/objc-methods.m index 0636447eb87e..9b8002c003a2 100644 --- a/clang/test/Analysis/copypaste/objc-methods.m +++ b/clang/test/Analysis/copypaste/objc-methods.m @@ -7,7 +7,7 @@ @end @implementation A -- (int) setOk : (int) a : (int) b { // expected-warning{{Detected code clone.}} +- (int) setOk : (int) a : (int) b { // expected-warning{{Duplicate code detected}} if (a > b) return a; return b; @@ -19,7 +19,7 @@ @end @implementation B -- (int) setOk : (int) a : (int) b { // expected-note{{Related code clone is here.}} +- (int) setOk : (int) a : (int) b { // expected-note{{Similar code here}} if (a > b) return a; return b; diff --git a/clang/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp b/clang/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp new file mode 100644 index 000000000000..1180d447a7a3 --- /dev/null +++ b/clang/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -analyze -analyzer-output=plist -analyzer-config notes-as-events=true -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s +// RUN: FileCheck --input-file=%t.plist %s + +void log(); + +int max(int a, int b) { // expected-warning{{Duplicate code detected}} + log(); + if (a > b) + return a; + return b; +} + +int maxClone(int a, int b) { // no-note (converted into event) + log(); + if (a > b) + return a; + return b; +} + +// CHECK: diagnostics +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line13 +// CHECK-NEXT: col28 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line13 +// CHECK-NEXT: col28 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line18 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Similar code here +// CHECK-NEXT: message +// CHECK-NEXT: Similar code here +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line11 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Duplicate code detected +// CHECK-NEXT: message +// CHECK-NEXT: Duplicate code detected +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionDuplicate code detected +// CHECK-NEXT: categoryCode clone +// CHECK-NEXT: typeExact code clone +// CHECK-NEXT: check_namealpha.clone.CloneChecker +// CHECK-NEXT: +// CHECK-NEXT: issue_hash_content_of_line_in_context3d15184f38c5fa57e479b744fe3f5035 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: diff --git a/clang/test/Analysis/copypaste/plist-diagnostics.cpp b/clang/test/Analysis/copypaste/plist-diagnostics.cpp new file mode 100644 index 000000000000..109d8e4fc71f --- /dev/null +++ b/clang/test/Analysis/copypaste/plist-diagnostics.cpp @@ -0,0 +1,71 @@ +// RUN: %clang_cc1 -analyze -analyzer-output=plist -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s +// RUN: FileCheck --input-file=%t.plist %s + +void log(); + +int max(int a, int b) { // expected-warning{{Duplicate code detected}} + log(); + if (a > b) + return a; + return b; +} + +int maxClone(int a, int b) { // expected-note{{Similar code here}} + log(); + if (a > b) + return a; + return b; +} + +// FIXME: This plist output doesn't include the extra note on line 13. +// It should be updated once the format for extra notes in plists is defined. + +// CHECK: diagnostics +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line11 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Duplicate code detected +// CHECK-NEXT: message +// CHECK-NEXT: Duplicate code detected +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionDuplicate code detected +// CHECK-NEXT: categoryCode clone +// CHECK-NEXT: typeExact code clone +// CHECK-NEXT: check_namealpha.clone.CloneChecker +// CHECK-NEXT: +// CHECK-NEXT: issue_hash_content_of_line_in_context3d15184f38c5fa57e479b744fe3f5035 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: diff --git a/clang/test/Analysis/copypaste/sub-sequences.cpp b/clang/test/Analysis/copypaste/sub-sequences.cpp index 59dc464ddc18..ff73632e4353 100644 --- a/clang/test/Analysis/copypaste/sub-sequences.cpp +++ b/clang/test/Analysis/copypaste/sub-sequences.cpp @@ -7,14 +7,14 @@ void log(); int max(int a, int b) { log2(a); - log(); // expected-warning{{Detected code clone.}} + log(); // expected-warning{{Duplicate code detected}} if (a > b) return a; return b; } int maxClone(int a, int b) { - log(); // expected-note{{Related code clone is here.}} + log(); // expected-note{{Similar code here}} if (a > b) return a; return b; diff --git a/clang/test/Analysis/copypaste/suspicious-clones.cpp b/clang/test/Analysis/copypaste/suspicious-clones.cpp index fb22d38c850e..c64a1dc8b8ff 100644 --- a/clang/test/Analysis/copypaste/suspicious-clones.cpp +++ b/clang/test/Analysis/copypaste/suspicious-clones.cpp @@ -8,14 +8,14 @@ int max(int a, int b) { log(); if (a > b) return a; - return b; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}} + return b; // expected-note{{Similar code using 'b' here}} } int maxClone(int x, int y, int z) { log(); if (x > y) return x; - return z; // expected-warning{{suspicious code clone detected; did you mean to use 'y'?}} + return z; // expected-warning{{Potential copy-paste error; did you really mean to use 'z' here?}} } // Tests finding a suspicious clone that references global variables. @@ -33,7 +33,7 @@ void busyIncrement() { while (true) { if (m1.try_lock()) { ++i; - m1.unlock(); // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}} + m1.unlock(); // expected-note{{Similar code using 'm1' here}} if (i > 1000) { return; } @@ -45,7 +45,7 @@ void faultyBusyIncrement() { while (true) { if (m1.try_lock()) { ++i; - m2.unlock(); // expected-warning{{suspicious code clone detected; did you mean to use 'm1'?}} + m2.unlock(); // expected-warning{{Potential copy-paste error; did you really mean to use 'm2' here?}} if (i > 1000) { return; } @@ -58,14 +58,14 @@ void faultyBusyIncrement() { int foo(int a, int b, int c) { a += b + c; b /= a + b; - c -= b * a; // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}} + c -= b * a; // expected-warning{{Potential copy-paste error; did you really mean to use 'b' here?}} return c; } int fooClone(int a, int b, int c) { a += b + c; b /= a + b; - c -= a * a; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} + c -= a * a; // expected-note{{Similar code using 'a' here}} return c; } @@ -77,21 +77,21 @@ int fooClone(int a, int b, int c) { long bar1(long a, long b, long c, long d) { c = a - b; c = c / d * a; - d = b * b - c; // expected-warning{{suspicious code clone detected; did you mean to use 'c'?}} + d = b * b - c; // expected-warning{{Potential copy-paste error; did you really mean to use 'b' here?}} return d; } long bar2(long a, long b, long c, long d) { c = a - b; c = c / d * a; - d = c * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} \ - // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}} + d = c * b - c; // expected-note{{Similar code using 'c' here}} \ + // expected-warning{{Potential copy-paste error; did you really mean to use 'c' here?}} return d; } long bar3(long a, long b, long c, long d) { c = a - b; c = c / d * a; - d = a * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'c'?}} + d = a * b - c; // expected-note{{Similar code using 'a' here}} return d; } diff --git a/clang/test/Analysis/copypaste/text-diagnostics.cpp b/clang/test/Analysis/copypaste/text-diagnostics.cpp new file mode 100644 index 000000000000..a80afdb1eaf8 --- /dev/null +++ b/clang/test/Analysis/copypaste/text-diagnostics.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -analyze -analyzer-output=text -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s + +void log(); + +int max(int a, int b) { // expected-warning{{Duplicate code detected}} // expected-note{{Duplicate code detected}} + log(); + if (a > b) + return a; + return b; +} + +int maxClone(int a, int b) { // expected-note{{Similar code here}} + log(); + if (a > b) + return a; + return b; +}