Charusso created this revision.
Charusso added reviewers: NoQ, alexfh, zinovy.nis, JonasToth, hokein, 
gribozavr, lebedev.ri.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny, 
dylanmckay.

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.

When the copy-paste happened the original authors were:
@alexfh, @zinovy.nis, @JonasToth, @hokein, @gribozavr, @lebedev.ri


Repository:
  rC Clang

https://reviews.llvm.org/D69746

Files:
  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/Analysis/check_analyzer_fixit.py
  clang/test/Analysis/virtualcall-fixit-remark.cpp
  clang/test/Analysis/virtualcall-fixit.cpp
  clang/test/Analysis/virtualcall-fixits.cpp
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===================================================================
--- clang/test/lit.cfg.py
+++ 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)
 
Index: clang/test/Analysis/virtualcall-fixit.cpp
===================================================================
--- /dev/null
+++ 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@-1 {{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+    // CHECK-FIXES: S::foo();
+  }
+  ~S();
+};
Index: clang/test/Analysis/check_analyzer_fixit.py
===================================================================
--- /dev/null
+++ clang/test/Analysis/check_analyzer_fixit.py
@@ -0,0 +1,137 @@
+#!/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 mostly copy-pasted from the Clang Tidy's 'check_clang_tidy.py'
+#  and some parts from the LLVM Lit's 'config.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 get_process_output(command):
+  try:
+    cmd = subprocess.Popen(
+      command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    stdout, stderr = cmd.communicate()
+    return (stdout, stderr)
+  except OSError:
+    print('Could not run process %s' % command)
+
+
+def get_clang_builtin_include_dir():
+  clang_dir, _ = get_process_output(['clang', '-print-file-name=include'])
+
+  clang_dir = clang_dir.strip()
+  if sys.platform in ['win32']:
+    clang_dir = clang_dir.replace('\\', '/')
+
+  return clang_dir
+
+
+def write_file(file_name, text):
+  with open(file_name, 'w') as f:
+    f.write(text)
+    f.truncate()
+
+
+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)
+
+  builtin_include_dir = get_clang_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 ' + repr(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()
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -10,6 +10,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
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
@@ -21,4 +21,6 @@
   clangLex
   clangStaticAnalyzerCheckers
   clangStaticAnalyzerCore
+  clangRewrite
+  clangToolingCore
   )
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ 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"
 
@@ -83,11 +87,14 @@
 namespace {
 class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
   DiagnosticsEngine &Diag;
-  bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false;
+  LangOptions LO;
+
+  bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false,
+       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"; }
 
@@ -101,6 +108,7 @@
   void enablePaths() { IncludePath = true; }
   void enableWerror() { ShouldEmitAsError = true; }
   void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
+  void enableFixItApplication() { ApplyFixIts = true; }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                             FilesMade *filesMade) override {
@@ -110,29 +118,42 @@
             : 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;
+      }
 
-    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();
-            }
-          }
-        };
+      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);
+
+          if (llvm::Error Err = Repls.add(Repl))
+            llvm_unreachable("An error occured during fix-it replacements");
+        }
+      }
+    };
 
     for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
          E = Diags.end();
@@ -164,6 +185,15 @@
                     Piece->getString(), Piece->getRanges(), Piece->getFixits());
       }
     }
+
+    if (!ApplyFixIts || Repls.empty())
+      return;
+
+    Rewriter Rewrite(SM, LO);
+    if (!applyAllReplacements(Repls, Rewrite))
+      llvm_unreachable("An error occured during applying fix-it replacements");
+
+    Rewrite.overwriteChangedFiles();
   }
 };
 } // end anonymous namespace
@@ -256,7 +286,7 @@
     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)
@@ -265,6 +295,9 @@
       if (Opts->ShouldEmitFixItHintsAsRemarks)
         clangDiags->enableFixitsAsRemarks();
 
+      if (Opts->ShouldApplyFixIts)
+        clangDiags->enableFixItApplication();
+
       if (Opts->AnalysisDiagOpt == PD_TEXT) {
         clangDiags->enablePaths();
 
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -304,6 +304,10 @@
                 "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.
 //===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to