MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: mitchell-stellar, klimek, owenpan.
MyDeveloperDay added projects: clang-format, clang-tools-extra.
Herald added a subscriber: mgorny.
Herald added a project: clang.

Related somewhat to D29039: [clang-format] Proposal for clang-format -r option 
<https://reviews.llvm.org/D29039>

On seeing a quote on twitter by @invalidop

> If it's not formatted with clang-format it's a build error.

This made me want to change the way I use clang-format into a tool that could 
optionally show me where my source code violates clang-format syle.

When I'm making a change to clang-format itself, one thing I like to do to test 
the change is to ensure I didn't cause a huge wave of changes, what I want to 
do is simply run this on a known formatted directory and see if any new 
differences arrive in a manner I'm used to.

This started me thinking that we should allow build systems to run clang-format 
on a whole tree and emit compiler style warnings about files that fail 
clang-format in a form that would make them as a warning in most build systems 
and because those build systems range in their construction I don't think its 
unreasonable to NOT expect them to have to do the directory searching or 
parsing the output replacements themselves, but simply transform that into an 
error code when there are changes required.

I am starting this by suggesing adding a -n or -dry-run command line argument 
which would emit a warning/error of the form

Support for various common compiler command line argumuments like '-Werror' and 
'-ferror-limit' could make this very flexible to be integrated into build 
systems and CI systems.

  > $ /usr/bin/clang-format --dry-run ClangFormat.cpp -ferror-limit=3 
-fcolor-diagnostics
  > ClangFormat.cpp:54:29: warning: code should be clang-formatted 
[-Wclang-format-violations]
  > static cl::list<std::string>
  >                             ^
  > ClangFormat.cpp:55:20: warning: code should be clang-formatted 
[-Wclang-format-violations]
  > LineRanges("lines", cl::desc("<start line>:<end line> - format a range of\n"
  >                    ^
  > ClangFormat.cpp:55:77: warning: code should be clang-formatted 
[-Wclang-format-violations]
  > LineRanges("lines", cl::desc("<start line>:<end line> - format a range of\n"
  >                                                                             
^


Repository:
  rC Clang

https://reviews.llvm.org/D68554

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/ClangFormat.cpp

Index: clang/tools/clang-format/ClangFormat.cpp
===================================================================
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -107,6 +108,54 @@
     Verbose("verbose", cl::desc("If set, shows the list of processed files"),
             cl::cat(ClangFormatCategory));
 
+// Use --dry-run to match other LLVM tools when you mean do it but don't
+// actually do it
+static cl::opt<bool>
+    DryRun("dry-run",
+           cl::desc("If set, do not actually make the formatting changes"),
+           cl::cat(ClangFormatCategory));
+
+// Use -n as a common command as an alias for --dry-run. (git and make use -n)
+static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
+                             cl::cat(ClangFormatCategory), cl::aliasopt(DryRun),
+                             cl::NotHidden);
+
+// Emulate being able to turn on/off the warning.
+static cl::opt<bool>
+    WarnFormat("Wclang-format-violations",
+               cl::desc("Warnings about individual formatting changes needed. "
+                        "Used only with --dry-run or -n"),
+               cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt<bool>
+    NoWarnFormat("Wno-clang-format-violations",
+                 cl::desc("Do not warn about individual formatting changes "
+                          "needed. Used only with --dry-run or -n"),
+                 cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt<unsigned> ErrorLimit(
+    "ferror-limit",
+    cl::desc("Set the maximum number of clang-format errors to emit before "
+             "stopping (0 = no limit). Used only with --dry-run or -n"),
+    cl::init(0), cl::cat(ClangFormatCategory));
+
+static cl::opt<bool>
+    WarningsAsErrors("Werror",
+                     cl::desc("If set, changes formatting warnings to errors"),
+                     cl::cat(ClangFormatCategory));
+
+static cl::opt<bool>
+    ShowColors("fcolor-diagnostics",
+               cl::desc("If set, and on a color-capable terminal controls "
+                        "whether or not to print diagnostics in color"),
+               cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt<bool>
+    NoShowColors("fno-color-diagnostics",
+                 cl::desc("If set, and on a color-capable terminal controls "
+                          "whether or not to print diagnostics in color"),
+                 cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
 static cl::list<std::string> FileNames(cl::Positional, cl::desc("[<file> ...]"),
                                        cl::cat(ClangFormatCategory));
 
@@ -313,23 +362,65 @@
   // Get new affected ranges after sorting `#includes`.
   Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
   FormattingAttemptStatus Status;
-  Replacements FormatChanges = reformat(*FormatStyle, *ChangedCode, Ranges,
-                                        AssumedFileName, &Status);
+  Replacements FormatChanges =
+      reformat(*FormatStyle, *ChangedCode, Ranges, AssumedFileName, &Status);
   Replaces = Replaces.merge(FormatChanges);
-  if (OutputXML) {
-    outs() << "<?xml version='1.0'?>\n<replacements "
-              "xml:space='preserve' incomplete_format='"
-           << (Status.FormatComplete ? "false" : "true") << "'";
-    if (!Status.FormatComplete)
-      outs() << " line='" << Status.Line << "'";
-    outs() << ">\n";
-    if (Cursor.getNumOccurrences() != 0)
-      outs() << "<cursor>"
-             << FormatChanges.getShiftedCodePosition(CursorPosition)
-             << "</cursor>\n";
-
-    outputReplacementsXML(Replaces);
-    outs() << "</replacements>\n";
+  if (OutputXML || DryRun) {
+    if (DryRun) {
+      if (!Replaces.empty()) {
+        IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
+            new DiagnosticOptions();
+        DiagOpts->ShowColors = (ShowColors && !NoShowColors);
+
+        TextDiagnosticPrinter *DiagsBuffer =
+            new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts, false);
+
+        IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+        IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
+            new DiagnosticsEngine(DiagID, &*DiagOpts, DiagsBuffer));
+
+        IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
+            new llvm::vfs::InMemoryFileSystem);
+        FileManager Files(FileSystemOptions(), InMemoryFileSystem);
+        SourceManager Sources(*Diags, Files);
+        FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
+                                           Files, InMemoryFileSystem.get());
+
+        const unsigned ID = Diags->getCustomDiagID(
+            WarningsAsErrors ? clang::DiagnosticsEngine::Error
+                             : clang::DiagnosticsEngine::Warning,
+            "code should be clang-formatted [-Wclang-format-violations]");
+
+        unsigned Errors = 0;
+        DiagsBuffer->BeginSourceFile(LangOptions(), nullptr);
+        if (WarnFormat && !NoWarnFormat) {
+          for (const auto &R : Replaces) {
+            Diags->Report(Sources.getLocForStartOfFile(FileID).getLocWithOffset(
+                              R.getOffset()),
+                          ID);
+            Errors++;
+            if (ErrorLimit && Errors >= ErrorLimit)
+              break;
+          }
+        }
+        DiagsBuffer->EndSourceFile();
+      }
+      return true;
+    } else {
+      outs() << "<?xml version='1.0'?>\n<replacements "
+                "xml:space='preserve' incomplete_format='"
+             << (Status.FormatComplete ? "false" : "true") << "'";
+      if (!Status.FormatComplete)
+        outs() << " line='" << Status.Line << "'";
+      outs() << ">\n";
+      if (Cursor.getNumOccurrences() != 0)
+        outs() << "<cursor>"
+               << FormatChanges.getShiftedCodePosition(CursorPosition)
+               << "</cursor>\n";
+
+      outputReplacementsXML(Replaces);
+      outs() << "</replacements>\n";
+    }
   } else {
     IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
         new llvm::vfs::InMemoryFileSystem);
Index: clang/tools/clang-format/CMakeLists.txt
===================================================================
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -7,6 +7,7 @@
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
+  clangFrontend
   clangRewrite
   clangToolingCore
   )
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to