Commit f69c74db authored by Charusso's avatar Charusso
Browse files

[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
parent cac06860

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

+4 −0
Original line number Diff line number Diff line
@@ -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.

//===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

+64 −26
Original line number Diff line number Diff line
@@ -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 @@ public: 
  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,28 +121,43 @@ public: 
            : Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");

    unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");

    unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");

    SourceManager &SM = Diag.getSourceManager();



    auto reportPiece =

        [&](unsigned ID, SourceLocation Loc, StringRef String,

            ArrayRef<SourceRange> Ranges, ArrayRef<FixItHint> Fixits) {

          if (!FixitsAsRemarks) {

    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;

          } else {

        return;

      }



      Diag.Report(Loc, ID) << String << Ranges;

      if (FixitsAsRemarks) {

        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 << "'";

          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::errs() << "Error applying replacement " << Repl.toString()

                         << ": " << Err << "\n";

          }

        }

      }

    };



    for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
@@ -165,6 +190,16 @@ public: 
                    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 @@ public: 
    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 @@ public: 
      if (Opts->ShouldEmitFixItHintsAsRemarks)

        clangDiags->enableFixitsAsRemarks();



      if (Opts->ShouldApplyFixIts)

        clangDiags->enableApplyFixIts();



      if (Opts->AnalysisDiagOpt == PD_TEXT) {

        clangDiags->enablePaths();

clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

+2 −0
Original line number Diff line number Diff line
@@ -21,4 +21,6 @@ add_clang_library(clangStaticAnalyzerFrontend 
  clangLex

  clangStaticAnalyzerCheckers

  clangStaticAnalyzerCore

  clangRewrite

  clangToolingCore

  )

clang/test/Analysis/analyzer-config.c

+2 −1
Original line number Diff line number Diff line
@@ -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

clang/test/Analysis/check-analyzer-fixit.py

0 → 100644
+121 −0
Original line number Diff line number Diff line 
#!/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()