[clang-format] NFC - Move functionality into functions to help code structure

[MyDeveloperDay]

Summary

I don't think its the main()'s responsibility to be working out how to dump the config, or the format()'s responsibility to determine what is and isn't a valid BOM

move those functions out to their own functions, this gives clear roles and responsibilities and it makes it easier to add new additional capabilities.

[owenpan]

clang/tools/clang-format/ClangFormat.cpp

245

May I suggest getInvalidBOM(const StringRef) for the function name and parameter type?

245–266

This code was copied from clang/lib/Basic/SourceManager.cpp. I suggest that you move this function to there and export it through clang/include/clang/Basic/SourceManager.h.

381

AssumeFileName is a file-scope global, so the file-scope function doesn't need to make it a parameter? Also, a bool or int may be a better return type than unsigned.

[MyDeveloperDay]

I'll likely abandon this review when D68554: [clang-format] Proposal for clang-format to give compiler style warnings lands

clang/tools/clang-format/ClangFormat.cpp

245

@klimek already got me on that one over in D68554: [clang-format] Proposal for clang-format to give compiler style warnings

245–266

That sounds good, do you mind if I do that separately? (as this review is a little entwined with
D68554: [clang-format] Proposal for clang-format to give compiler style warnings, which is where I'll actually update that revision with your suggestions.

381

I think it should be int as it ends up being the return from main, I'll do that.

[MyDeveloperDay]

Review comments regarding movign getInvalidBOM are addressed in D68914: [clang-format] Remove duplciate code from Invalid BOM detection

clang/tools/clang-format/ClangFormat.cpp

for (const auto &R : Replaces) {
     outs() << "<replacement "
            << "offset='" << R.getOffset() << "' "
            << "length='" << R.getLength() << "'>";
     outputReplacementXML(R.getReplacementText());
     outs() << "</replacement>\n";
   }
 }
 
+// Returns an invalid BOM
+static const char *hasInValidBOM(StringRef BufStr) {
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  const char *InvalidBOM =
+      llvm::StringSwitch<const char *>(BufStr)
+          .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+                      "UTF-32 (BE)")
+          .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+                      "UTF-32 (LE)")
+          .StartsWith("\xFE\xFF", "UTF-16 (BE)")
+          .StartsWith("\xFF\xFE", "UTF-16 (LE)")
+          .StartsWith("\x2B\x2F\x76", "UTF-7")
+          .StartsWith("\xF7\x64\x4C", "UTF-1")
+          .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+          .StartsWith("\x0E\xFE\xFF", "SCSU")
+          .StartsWith("\xFB\xEE\x28", "BOCU-1")
+          .StartsWith("\x84\x31\x95\x33", "GB-18030")
+          .Default(nullptr);
+  return InvalidBOM;
+}
+
 // Returns true on error.
 static bool format(StringRef FileName) {
   if (!OutputXML && Inplace && FileName == "-") {
     errs() << "error: cannot use -i when reading from stdin.\n";
     return false;
   }
   // On Windows, overwriting a file with an open file mapping doesn't work,
   // so read the whole file into memory when formatting in-place.
   ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
       !OutputXML && Inplace ? MemoryBuffer::getFileAsStream(FileName)
                             : MemoryBuffer::getFileOrSTDIN(FileName);
   if (std::error_code EC = CodeOrErr.getError()) {
     errs() << EC.message() << "\n";
     return true;
   }
   std::unique_ptr<llvm::MemoryBuffer> Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
     return false; // Empty files are formatted correctly.
 
-  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
-  // We only support UTF-8 with and without a BOM right now.  See
-  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
-  // for more information.
   StringRef BufStr = Code->getBuffer();
-  const char *InvalidBOM =
-      llvm::StringSwitch<const char *>(BufStr)
-          .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
-                      "UTF-32 (BE)")
-          .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
-                      "UTF-32 (LE)")
-          .StartsWith("\xFE\xFF", "UTF-16 (BE)")
-          .StartsWith("\xFF\xFE", "UTF-16 (LE)")
-          .StartsWith("\x2B\x2F\x76", "UTF-7")
-          .StartsWith("\xF7\x64\x4C", "UTF-1")
-          .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-          .StartsWith("\x0E\xFE\xFF", "SCSU")
-          .StartsWith("\xFB\xEE\x28", "BOCU-1")
-          .StartsWith("\x84\x31\x95\x33", "GB-18030")
-          .Default(nullptr);
+
+  const char *InvalidBOM = hasInValidBOM(BufStr);
 
   if (InvalidBOM) {
     errs() << "error: encoding with unsupported byte order mark \""
            << InvalidBOM << "\" detected";
     if (FileName != "-")
       errs() << " in file '" << FileName << "'";
     errs() << ".\n";
     return true;

 
 } // namespace format
 } // namespace clang
 
 static void PrintVersion(raw_ostream &OS) {
   OS << clang::getClangToolFullVersion("clang-format") << '\n';
 }
 
-int main(int argc, const char **argv) {
-  llvm::InitLLVM X(argc, argv);
-
-  cl::HideUnrelatedOptions(ClangFormatCategory);
-
-  cl::SetVersionPrinter(PrintVersion);
-  cl::ParseCommandLineOptions(
-      argc, argv,
-      "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.\n\n"
-      "If no arguments are specified, it formats the code from standard input\n"
-      "and writes the result to the standard output.\n"
-      "If <file>s are given, it reformats the files. If -i is specified\n"
-      "together with <file>s, the files are edited in-place. Otherwise, the\n"
-      "result is written to the standard output.\n");
-
-  if (Help) {
-    cl::PrintHelpMessage();
-    return 0;
-  }
-
-  if (DumpConfig) {
-    StringRef FileName;
-    std::unique_ptr<llvm::MemoryBuffer> Code;
-    if (FileNames.empty()) {
-      // We can't read the code to detect the language if there's no
-      // file name, so leave Code empty here.
-      FileName = AssumeFileName;
-    } else {
-      // Read in the code in case the filename alone isn't enough to
-      // detect the language.
-      ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
-          MemoryBuffer::getFileOrSTDIN(FileNames[0]);
-      if (std::error_code EC = CodeOrErr.getError()) {
-        llvm::errs() << EC.message() << "\n";
-        return 1;
-      }
-      FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
-      Code = std::move(CodeOrErr.get());
-    }
-    llvm::Expected<clang::format::FormatStyle> FormatStyle =
-        clang::format::getStyle(Style, FileName, FallbackStyle,
-                                Code ? Code->getBuffer() : "");
-    if (!FormatStyle) {
-      llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
-      return 1;
-    }
-    std::string Config = clang::format::configurationAsText(*FormatStyle);
-    outs() << Config << "\n";
-    return 0;
-  }
+// Dump the configuration.
+static unsigned dumpConfig(StringRef AssumeFileName) {
 
+int main(int argc, const char **argv) {
+  llvm::InitLLVM X(argc, argv);
+
+  cl::HideUnrelatedOptions(ClangFormatCategory);
+
+  cl::SetVersionPrinter(PrintVersion);
+  cl::ParseCommandLineOptions(
+      argc, argv,
+      "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.\n\n"
+      "If no arguments are specified, it formats the code from standard input\n"
+      "and writes the result to the standard output.\n"
+      "If <file>s are given, it reformats the files. If -i is specified\n"
+      "together with <file>s, the files are edited in-place. Otherwise, the\n"
+      "result is written to the standard output.\n");
+
+  if (Help) {
+    cl::PrintHelpMessage();
+    return 0;
+  }
+
+  if (DumpConfig) {
+    return dumpConfig(AssumeFileName);
+  }
+
   bool Error = false;
   if (FileNames.empty()) {
     Error = clang::format::format("-");
     return Error ? 1 : 0;
   }
   if (FileNames.size() != 1 &&
       (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty())) {
     errs() << "error: -offset, -length and -lines can only be used for "