[clang] f69c74d - [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 3 21:27:47 PST 2020
Author: Charusso
Date: 2020-03-04T06:26:33+01:00
New Revision: f69c74db34f42c20c167b8fb0f93ec05a0e77b45
URL: https://github.com/llvm/llvm-project/commit/f69c74db34f42c20c167b8fb0f93ec05a0e77b45
DIFF: https://github.com/llvm/llvm-project/commit/f69c74db34f42c20c167b8fb0f93ec05a0e77b45.diff
LOG: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script
Summary:
This patch introduces a way to apply the fix-its by the Analyzer:
`-analyzer-config apply-fixits=true`.
The fix-its should be testable, therefore I have copied the well-tested
`check_clang_tidy.py` script. The idea is that the Analyzer's workflow
is different so it would be very difficult to use only one script for
both Tidy and the Analyzer, the script would diverge a lot.
Example test: `// RUN: %check-analyzer-fixit %s %t -analyzer-checker=core`
When the copy-paste happened the original authors were:
@alexfh, @zinovy.nis, @JonasToth, @hokein, @gribozavr, @lebedev.ri
Reviewed By: NoQ, alexfh, zinovy.nis
Differential Revision: https://reviews.llvm.org/D69746
Added:
clang/test/Analysis/check-analyzer-fixit.py
clang/test/Analysis/virtualcall-fixit.cpp
Modified:
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/test/Analysis/analyzer-config.c
clang/test/lit.cfg.py
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index 00febf688195..400e953e3c6c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -310,6 +310,10 @@ ANALYZER_OPTION(bool, ShouldEmitFixItHintsAsRemarks, "fixits-as-remarks",
"Emit fix-it hints as remarks for testing purposes",
false)
+ANALYZER_OPTION(bool, ShouldApplyFixIts, "apply-fixits",
+ "Apply the fix-it hints to the files",
+ false)
+
//===----------------------------------------------------------------------===//
// Unsigned analyzer options.
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 28a3ab2ba188..1e036dba2de5 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -12,7 +12,6 @@
#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
#include "ModelInjector.h"
-#include "clang/Analysis/PathDiagnostic.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
@@ -21,10 +20,12 @@
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CallGraph.h"
#include "clang/Analysis/CodeInjector.h"
+#include "clang/Analysis/PathDiagnostic.h"
#include "clang/Basic/SourceManager.h"
#include "clang/CrossTU/CrossTranslationUnit.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/StaticAnalyzer/Checkers/LocalCheckers.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
@@ -33,6 +34,8 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Support/FileSystem.h"
@@ -46,6 +49,7 @@
using namespace clang;
using namespace ento;
+using namespace tooling;
#define DEBUG_TYPE "AnalysisConsumer"
@@ -84,11 +88,16 @@ void ento::createTextPathDiagnosticConsumer(
namespace {
class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
DiagnosticsEngine &Diag;
- bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false;
+ LangOptions LO;
+
+ bool IncludePath = false;
+ bool ShouldEmitAsError = false;
+ bool FixitsAsRemarks = false;
+ bool ApplyFixIts = false;
public:
- ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag)
- : Diag(Diag) {}
+ ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag, LangOptions LO)
+ : Diag(Diag), LO(LO) {}
~ClangDiagPathDiagConsumer() override {}
StringRef getName() const override { return "ClangDiags"; }
@@ -102,6 +111,7 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
void enablePaths() { IncludePath = true; }
void enableWerror() { ShouldEmitAsError = true; }
void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
+ void enableApplyFixIts() { ApplyFixIts = true; }
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
FilesMade *filesMade) override {
@@ -111,29 +121,44 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
: Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
+ SourceManager &SM = Diag.getSourceManager();
+
+ Replacements Repls;
+ auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
+ ArrayRef<SourceRange> Ranges,
+ ArrayRef<FixItHint> Fixits) {
+ if (!FixitsAsRemarks && !ApplyFixIts) {
+ Diag.Report(Loc, ID) << String << Ranges << Fixits;
+ return;
+ }
+
+ Diag.Report(Loc, ID) << String << Ranges;
+ if (FixitsAsRemarks) {
+ for (const FixItHint &Hint : Fixits) {
+ llvm::SmallString<128> Str;
+ llvm::raw_svector_ostream OS(Str);
+ // FIXME: Add support for InsertFromRange and
+ // BeforePreviousInsertion.
+ assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
+ assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
+ OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) << "-"
+ << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) << ": '"
+ << Hint.CodeToInsert << "'";
+ Diag.Report(Loc, RemarkID) << OS.str();
+ }
+ }
+
+ if (ApplyFixIts) {
+ for (const FixItHint &Hint : Fixits) {
+ Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
- auto reportPiece =
- [&](unsigned ID, SourceLocation Loc, StringRef String,
- ArrayRef<SourceRange> Ranges, ArrayRef<FixItHint> Fixits) {
- if (!FixitsAsRemarks) {
- Diag.Report(Loc, ID) << String << Ranges << Fixits;
- } else {
- Diag.Report(Loc, ID) << String << Ranges;
- for (const FixItHint &Hint : Fixits) {
- SourceManager &SM = Diag.getSourceManager();
- llvm::SmallString<128> Str;
- llvm::raw_svector_ostream OS(Str);
- // FIXME: Add support for InsertFromRange and
- // BeforePreviousInsertion.
- assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
- assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
- OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin())
- << "-" << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd())
- << ": '" << Hint.CodeToInsert << "'";
- Diag.Report(Loc, RemarkID) << OS.str();
- }
+ if (llvm::Error Err = Repls.add(Repl)) {
+ llvm::errs() << "Error applying replacement " << Repl.toString()
+ << ": " << Err << "\n";
}
- };
+ }
+ }
+ };
for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
E = Diags.end();
@@ -165,6 +190,16 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
Piece->getString(), Piece->getRanges(), Piece->getFixits());
}
}
+
+ if (!ApplyFixIts || Repls.empty())
+ return;
+
+ Rewriter Rewrite(SM, LO);
+ if (!applyAllReplacements(Repls, Rewrite)) {
+ llvm::errs() << "An error occured during applying fix-it.\n";
+ }
+
+ Rewrite.overwriteChangedFiles();
}
};
} // end anonymous namespace
@@ -257,7 +292,7 @@ class AnalysisConsumer : public AnalysisASTConsumer,
if (Opts->AnalysisDiagOpt != PD_NONE) {
// Create the PathDiagnosticConsumer.
ClangDiagPathDiagConsumer *clangDiags =
- new ClangDiagPathDiagConsumer(PP.getDiagnostics());
+ new ClangDiagPathDiagConsumer(PP.getDiagnostics(), PP.getLangOpts());
PathConsumers.push_back(clangDiags);
if (Opts->AnalyzerWerror)
@@ -266,6 +301,9 @@ class AnalysisConsumer : public AnalysisASTConsumer,
if (Opts->ShouldEmitFixItHintsAsRemarks)
clangDiags->enableFixitsAsRemarks();
+ if (Opts->ShouldApplyFixIts)
+ clangDiags->enableApplyFixIts();
+
if (Opts->AnalysisDiagOpt == PD_TEXT) {
clangDiags->enablePaths();
diff --git a/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt b/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
index 5e7dd8f18cd7..6f1151ab0c11 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
@@ -21,4 +21,6 @@ add_clang_library(clangStaticAnalyzerFrontend
clangLex
clangStaticAnalyzerCheckers
clangStaticAnalyzerCore
+ clangRewrite
+ clangToolingCore
)
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 5b0566d5a820..2e4af7109125 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -11,6 +11,7 @@
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
+// CHECK-NEXT: apply-fixits = false
// CHECK-NEXT: avoid-suppressing-null-argument-paths = false
// CHECK-NEXT: c++-allocator-inlining = true
// CHECK-NEXT: c++-container-inlining = false
@@ -100,4 +101,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 97
+// CHECK-NEXT: num-entries = 98
diff --git a/clang/test/Analysis/check-analyzer-fixit.py b/clang/test/Analysis/check-analyzer-fixit.py
new file mode 100644
index 000000000000..1d159aedec91
--- /dev/null
+++ b/clang/test/Analysis/check-analyzer-fixit.py
@@ -0,0 +1,121 @@
+#!/usr/bin/env python
+#
+#===- check-analyzer-fixit.py - Static Analyzer test helper ---*- python -*-===#
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===------------------------------------------------------------------------===#
+#
+# This file copy-pasted mostly from the Clang-Tidy's 'check_clang_tidy.py'.
+#
+#===------------------------------------------------------------------------===#
+
+r"""
+Clang Static Analyzer test helper
+=================================
+
+This script runs the Analyzer in fix-it mode and verify fixes, warnings, notes.
+
+Usage:
+ check-analyzer-fixit.py <source-file> <temp-file> [analyzer arguments]
+
+Example:
+ // RUN: %check-analyzer-fixit %s %t -analyzer-checker=core
+"""
+
+import argparse
+import os
+import re
+import subprocess
+import sys
+
+
+def write_file(file_name, text):
+ with open(file_name, 'w') as f:
+ f.write(text)
+
+
+def run_test_once(args, extra_args):
+ input_file_name = args.input_file_name
+ temp_file_name = args.temp_file_name
+ clang_analyzer_extra_args = extra_args
+
+ file_name_with_extension = input_file_name
+ _, extension = os.path.splitext(file_name_with_extension)
+ if extension not in ['.c', '.hpp', '.m', '.mm']:
+ extension = '.cpp'
+ temp_file_name = temp_file_name + extension
+
+ with open(input_file_name, 'r') as input_file:
+ input_text = input_file.read()
+
+ # Remove the contents of the CHECK lines to avoid CHECKs matching on
+ # themselves. We need to keep the comments to preserve line numbers while
+ # avoiding empty lines which could potentially trigger formatting-related
+ # checks.
+ cleaned_test = re.sub('// *CHECK-[A-Z0-9\-]*:[^\r\n]*', '//', input_text)
+ write_file(temp_file_name, cleaned_test)
+
+ original_file_name = temp_file_name + ".orig"
+ write_file(original_file_name, cleaned_test)
+
+ try:
+ builtin_include_dir = subprocess.check_output(
+ ['clang', '-print-file-name=include'], stderr=subprocess.STDOUT)
+ except subprocess.CalledProcessError as e:
+ print('Cannot print Clang include directory: ' + e.output.decode())
+
+ builtin_include_dir = os.path.normpath(builtin_include_dir)
+
+ args = (['clang', '-cc1', '-internal-isystem', builtin_include_dir,
+ '-nostdsysteminc', '-analyze', '-analyzer-constraints=range',
+ '-analyzer-config', 'apply-fixits=true']
+ + clang_analyzer_extra_args + ['-verify', temp_file_name])
+
+ print('Running ' + str(args) + '...')
+
+ try:
+ clang_analyzer_output = \
+ subprocess.check_output(args, stderr=subprocess.STDOUT).decode()
+ except subprocess.CalledProcessError as e:
+ print('Clang Static Analyzer test failed:\n' + e.output.decode())
+ raise
+
+ print('----------------- Clang Static Analyzer output -----------------\n' +
+ clang_analyzer_output +
+ '\n--------------------------------------------------------------')
+
+ try:
+
diff _output = subprocess.check_output(
+ ['
diff ', '-u', original_file_name, temp_file_name],
+ stderr=subprocess.STDOUT)
+ except subprocess.CalledProcessError as e:
+
diff _output = e.output
+
+ print('----------------------------- Fixes ----------------------------\n' +
+
diff _output.decode() +
+ '\n--------------------------------------------------------------')
+
+ try:
+ subprocess.check_output(
+ ['FileCheck', '-input-file=' + temp_file_name, input_file_name,
+ '-check-prefixes=CHECK-FIXES', '-strict-whitespace'],
+ stderr=subprocess.STDOUT)
+ except subprocess.CalledProcessError as e:
+ print('FileCheck failed:\n' + e.output.decode())
+ raise
+
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument('input_file_name')
+ parser.add_argument('temp_file_name')
+
+ args, extra_args = parser.parse_known_args()
+ run_test_once(args, extra_args)
+
+
+if __name__ == '__main__':
+ main()
diff --git a/clang/test/Analysis/virtualcall-fixit.cpp b/clang/test/Analysis/virtualcall-fixit.cpp
new file mode 100644
index 000000000000..e64e63ab0856
--- /dev/null
+++ b/clang/test/Analysis/virtualcall-fixit.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_analyzer_fixit %s %t \
+// RUN: -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true
+
+struct S {
+ virtual void foo();
+ S() {
+ foo();
+ // expected-warning at -1 {{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+ // CHECK-FIXES: S::foo();
+ }
+ ~S();
+};
diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 1ffb6d094d72..08d15edac94c 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -77,6 +77,11 @@
if config.clang_staticanalyzer_z3 == '1':
config.available_features.add('z3')
+ check_analyzer_fixit_path = os.path.join(
+ config.test_source_root, "Analysis", "check-analyzer-fixit.py")
+ config.substitutions.append(
+ ('%check_analyzer_fixit',
+ '%s %s' % (config.python_executable, check_analyzer_fixit_path)))
llvm_config.add_tool_substitutions(tools, tool_dirs)
More information about the cfe-commits
mailing list