diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index 6c6458b33d85423c18bf7ec1ef369ea00ff9f1f9..58a1bfbbb69ebd9348c7bfda3852b023af006259 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -853,17 +853,19 @@ extern const char *StyleOptionHelpDescription; /// \param[in] FileName Path to start search for .clang-format if ``StyleName`` /// == "file". /// \param[in] FallbackStyle The name of a predefined style used to fallback to -/// in case the style can't be determined from \p StyleName. +/// in case \p StyleName is "file" and no file can be found. /// \param[in] Code The actual code to be formatted. Used to determine the /// language if the filename isn't sufficient. /// \param[in] FS The underlying file system, in which the file resides. By /// default, the file system is the real file system. /// -/// \returns FormatStyle as specified by ``StyleName``. If no style could be -/// determined, the default is LLVM Style (see ``getLLVMStyle()``). -FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle, StringRef Code = "", - vfs::FileSystem *FS = nullptr); +/// \returns FormatStyle as specified by ``StyleName``. If ``StyleName`` is +/// "file" and no file is found, returns ``FallbackStyle``. If no style could be +/// determined, returns an Error. +llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, + StringRef FallbackStyle, + StringRef Code = "", + vfs::FileSystem *FS = nullptr); // \brief Returns a string representation of ``Language``. inline StringRef getLanguageName(FormatStyle::LanguageKind Language) { diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 389761d4824986738e47d8812267bc829a1fd315..e793d003ff61f3fb399bbedd6cbb012de38a8417 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -421,6 +421,11 @@ std::error_code make_error_code(ParseError e) { return std::error_code(static_cast<int>(e), getParseCategory()); } +inline llvm::Error make_string_error(const llvm::Twine &Message) { + return llvm::make_error<llvm::StringError>(Message, + llvm::inconvertibleErrorCode()); +} + const char *ParseErrorCategory::name() const noexcept { return "clang-format.parse_error"; } @@ -1882,9 +1887,9 @@ static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) { return FormatStyle::LK_Cpp; } -FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle, StringRef Code, - vfs::FileSystem *FS) { +llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, + StringRef FallbackStyle, StringRef Code, + vfs::FileSystem *FS) { if (!FS) { FS = vfs::getRealFileSystem().get(); } @@ -1898,35 +1903,28 @@ FormatStyle getStyle(StringRef StyleName, StringRef FileName, (Code.contains("\n- (") || Code.contains("\n+ ("))) Style.Language = FormatStyle::LK_ObjC; - if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { - llvm::errs() << "Invalid fallback style \"" << FallbackStyle - << "\" using LLVM style\n"; - return Style; - } + // FIXME: If FallbackStyle is explicitly "none", format is disabled. + if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) + return make_string_error("Invalid fallback style \"" + FallbackStyle.str()); if (StyleName.startswith("{")) { // Parse YAML/JSON style from the command line. - if (std::error_code ec = parseConfiguration(StyleName, &Style)) { - llvm::errs() << "Error parsing -style: " << ec.message() << ", using " - << FallbackStyle << " style\n"; - } + if (std::error_code ec = parseConfiguration(StyleName, &Style)) + return make_string_error("Error parsing -style: " + ec.message()); return Style; } if (!StyleName.equals_lower("file")) { if (!getPredefinedStyle(StyleName, Style.Language, &Style)) - llvm::errs() << "Invalid value for -style, using " << FallbackStyle - << " style\n"; + return make_string_error("Invalid value for -style"); return Style; } // Look for .clang-format/_clang-format file in the file's parent directories. SmallString<128> UnsuitableConfigFiles; SmallString<128> Path(FileName); - if (std::error_code EC = FS->makeAbsolute(Path)) { - llvm::errs() << EC.message() << "\n"; - return Style; - } + if (std::error_code EC = FS->makeAbsolute(Path)) + return make_string_error(EC.message()); for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { @@ -1943,25 +1941,23 @@ FormatStyle getStyle(StringRef StyleName, StringRef FileName, DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); Status = FS->status(ConfigFile.str()); - bool IsFile = + bool FoundConfigFile = Status && (Status->getType() == llvm::sys::fs::file_type::regular_file); - if (!IsFile) { + if (!FoundConfigFile) { // Try _clang-format too, since dotfiles are not commonly used on Windows. ConfigFile = Directory; llvm::sys::path::append(ConfigFile, "_clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); Status = FS->status(ConfigFile.str()); - IsFile = Status && - (Status->getType() == llvm::sys::fs::file_type::regular_file); + FoundConfigFile = Status && (Status->getType() == + llvm::sys::fs::file_type::regular_file); } - if (IsFile) { + if (FoundConfigFile) { llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = FS->getBufferForFile(ConfigFile.str()); - if (std::error_code EC = Text.getError()) { - llvm::errs() << EC.message() << "\n"; - break; - } + if (std::error_code EC = Text.getError()) + return make_string_error(EC.message()); if (std::error_code ec = parseConfiguration(Text.get()->getBuffer(), &Style)) { if (ec == ParseError::Unsuitable) { @@ -1970,19 +1966,17 @@ FormatStyle getStyle(StringRef StyleName, StringRef FileName, UnsuitableConfigFiles.append(ConfigFile); continue; } - llvm::errs() << "Error reading " << ConfigFile << ": " << ec.message() - << "\n"; - break; + return make_string_error("Error reading " + ConfigFile + ": " + + ec.message()); } DEBUG(llvm::dbgs() << "Using configuration file " << ConfigFile << "\n"); return Style; } } - if (!UnsuitableConfigFiles.empty()) { - llvm::errs() << "Configuration file(s) do(es) not support " - << getLanguageName(Style.Language) << ": " - << UnsuitableConfigFiles << "\n"; - } + if (!UnsuitableConfigFiles.empty()) + return make_string_error("Configuration file(s) do(es) not support " + + getLanguageName(Style.Language) + ": " + + UnsuitableConfigFiles); return Style; } diff --git a/lib/Tooling/Refactoring.cpp b/lib/Tooling/Refactoring.cpp index 308c1ac48b2813acde7ec913722de61059d4ca70..954a473f07ca0f0dcec0972d9a6796949ff64f41 100644 --- a/lib/Tooling/Refactoring.cpp +++ b/lib/Tooling/Refactoring.cpp @@ -68,8 +68,8 @@ int RefactoringTool::saveRewrittenFiles(Rewriter &Rewrite) { } bool formatAndApplyAllReplacements( - const std::map<std::string, Replacements> &FileToReplaces, Rewriter &Rewrite, - StringRef Style) { + const std::map<std::string, Replacements> &FileToReplaces, + Rewriter &Rewrite, StringRef Style) { SourceManager &SM = Rewrite.getSourceMgr(); FileManager &Files = SM.getFileManager(); @@ -83,9 +83,14 @@ bool formatAndApplyAllReplacements( FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User); StringRef Code = SM.getBufferData(ID); - format::FormatStyle CurStyle = format::getStyle(Style, FilePath, "LLVM"); + auto CurStyle = format::getStyle(Style, FilePath, "LLVM"); + if (!CurStyle) { + llvm::errs() << llvm::toString(CurStyle.takeError()) << "\n"; + return false; + } + auto NewReplacements = - format::formatReplacements(Code, CurReplaces, CurStyle); + format::formatReplacements(Code, CurReplaces, *CurStyle); if (!NewReplacements) { llvm::errs() << llvm::toString(NewReplacements.takeError()) << "\n"; return false; diff --git a/test/Format/style-on-command-line.cpp b/test/Format/style-on-command-line.cpp index 08da60a988d163062542882982a81aba183010dc..0d2d69206dc153e17bbb454f2ec95c666d6a9edd 100644 --- a/test/Format/style-on-command-line.cpp +++ b/test/Format/style-on-command-line.cpp @@ -1,11 +1,11 @@ // RUN: clang-format -style="{BasedOnStyle: Google, IndentWidth: 8}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s // RUN: clang-format -style="{BasedOnStyle: LLVM, IndentWidth: 7}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s -// RUN: clang-format -style="{BasedOnStyle: invalid, IndentWidth: 7}" -fallback-style=LLVM %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK3 %s -// RUN: clang-format -style="{lsjd}" %s -fallback-style=LLVM 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK4 %s +// RUN: not clang-format -style="{BasedOnStyle: invalid, IndentWidth: 7}" -fallback-style=LLVM %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK3 %s +// RUN: not clang-format -style="{lsjd}" %s -fallback-style=LLVM 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK4 %s // RUN: printf "BasedOnStyle: google\nIndentWidth: 5\n" > %T/.clang-format // RUN: clang-format -style=file -assume-filename=%T/foo.cpp < %s | FileCheck -strict-whitespace -check-prefix=CHECK5 %s // RUN: printf "\n" > %T/.clang-format -// RUN: clang-format -style=file -fallback-style=webkit -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK6 %s +// RUN: not clang-format -style=file -fallback-style=webkit -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK6 %s // RUN: rm %T/.clang-format // RUN: printf "BasedOnStyle: google\nIndentWidth: 6\n" > %T/_clang-format // RUN: clang-format -style=file -assume-filename=%T/foo.cpp < %s | FileCheck -strict-whitespace -check-prefix=CHECK7 %s @@ -15,13 +15,10 @@ void f() { // CHECK1: {{^ int\* i;$}} // CHECK2: {{^ int \*i;$}} // CHECK3: Unknown value for BasedOnStyle: invalid -// CHECK3: Error parsing -style: {{I|i}}nvalid argument, using LLVM style -// CHECK3: {{^ int \*i;$}} -// CHECK4: Error parsing -style: {{I|i}}nvalid argument, using LLVM style -// CHECK4: {{^ int \*i;$}} +// CHECK3: Error parsing -style: {{I|i}}nvalid argument +// CHECK4: Error parsing -style: {{I|i}}nvalid argument // CHECK5: {{^ int\* i;$}} // CHECK6: {{^Error reading .*\.clang-format: (I|i)nvalid argument}} -// CHECK6: {{^ int\* i;$}} // CHECK7: {{^ int\* i;$}} // CHECK8: {{^ int\* i;$}} // CHECK9: {{^ int \*i;$}} diff --git a/tools/clang-format/ClangFormat.cpp b/tools/clang-format/ClangFormat.cpp index 6c50daf538344bf78cefb1c852f6da9c570df3ce..b08d4fa4c186dbee8cd68fb6e83db6b60c48f2df 100644 --- a/tools/clang-format/ClangFormat.cpp +++ b/tools/clang-format/ClangFormat.cpp @@ -249,12 +249,17 @@ static bool format(StringRef FileName) { if (fillRanges(Code.get(), Ranges)) return true; StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName; - FormatStyle FormatStyle = + + llvm::Expected<FormatStyle> FormatStyle = getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer()); + if (!FormatStyle) { + llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n"; + return true; + } if (SortIncludes.getNumOccurrences() != 0) - FormatStyle.SortIncludes = SortIncludes; + FormatStyle->SortIncludes = SortIncludes; unsigned CursorPosition = Cursor; - Replacements Replaces = sortIncludes(FormatStyle, Code->getBuffer(), Ranges, + Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges, AssumedFileName, &CursorPosition); auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces); if (!ChangedCode) { @@ -264,7 +269,7 @@ static bool format(StringRef FileName) { // Get new affected ranges after sorting `#includes`. Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges); bool IncompleteFormat = false; - Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges, + Replacements FormatChanges = reformat(*FormatStyle, *ChangedCode, Ranges, AssumedFileName, &IncompleteFormat); Replaces = Replaces.merge(FormatChanges); if (OutputXML) { @@ -334,10 +339,15 @@ int main(int argc, const char **argv) { cl::PrintHelpMessage(); if (DumpConfig) { - std::string Config = - clang::format::configurationAsText(clang::format::getStyle( + llvm::Expected<clang::format::FormatStyle> FormatStyle = + clang::format::getStyle( Style, FileNames.empty() ? AssumeFileName : FileNames[0], - FallbackStyle)); + FallbackStyle); + if (!FormatStyle) { + llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n"; + return 1; + } + std::string Config = clang::format::configurationAsText(*FormatStyle); outs() << Config << "\n"; return 0; } diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 9bf7fb060f7837e97d6e38632d206b5829b2fcd3..38095402f0c948d79568412a196e6b02075bd7ff 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -10975,13 +10975,15 @@ TEST(FormatStyle, GetStyleOfFile) { ASSERT_TRUE( FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); auto Style1 = getStyle("file", "/a/.clang-format", "Google", "", &FS); - ASSERT_EQ(Style1, getLLVMStyle()); + ASSERT_TRUE((bool)Style1); + ASSERT_EQ(*Style1, getLLVMStyle()); // Test 2: fallback to default. ASSERT_TRUE( FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", "", &FS); - ASSERT_EQ(Style2, getMozillaStyle()); + ASSERT_TRUE((bool)Style2); + ASSERT_EQ(*Style2, getMozillaStyle()); // Test 3: format file in parent directory. ASSERT_TRUE( @@ -10990,7 +10992,34 @@ TEST(FormatStyle, GetStyleOfFile) { ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", "", &FS); - ASSERT_EQ(Style3, getGoogleStyle()); + ASSERT_TRUE((bool)Style3); + ASSERT_EQ(*Style3, getGoogleStyle()); + + // Test 4: error on invalid fallback style + auto Style4 = getStyle("file", "a.h", "KungFu", "", &FS); + ASSERT_FALSE((bool)Style4); + llvm::consumeError(Style4.takeError()); + + // Test 5: error on invalid yaml on command line + auto Style5 = getStyle("{invalid_key=invalid_value}", "a.h", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style5); + llvm::consumeError(Style5.takeError()); + + // Test 6: error on invalid style + auto Style6 = getStyle("KungFu", "a.h", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style6); + llvm::consumeError(Style6.takeError()); + + // Test 7: found config file, error on parsing it + ASSERT_TRUE( + FS.addFile("/d/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM\n" + "InvalidKey: InvalidValue"))); + ASSERT_TRUE( + FS.addFile("/d/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style7 = getStyle("file", "/d/.clang-format", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style7); + llvm::consumeError(Style7.takeError()); } TEST_F(ReplacementTest, FormatCodeAfterReplacements) { diff --git a/unittests/Format/FormatTestObjC.cpp b/unittests/Format/FormatTestObjC.cpp index 6a530f921e6dffa770d68263ecff8c4b63ec437d..680ff92f75e89e25acf14afa870de0b12dc0cb79 100644 --- a/unittests/Format/FormatTestObjC.cpp +++ b/unittests/Format/FormatTestObjC.cpp @@ -68,17 +68,21 @@ protected: FormatStyle Style; }; -TEST_F(FormatTestObjC, DetectsObjCInHeaders) { - Style = getStyle("LLVM", "a.h", "none", "@interface\n" - "- (id)init;"); - EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language); +TEST(FormatTestObjCStyle, DetectsObjCInHeaders) { + auto Style = getStyle("LLVM", "a.h", "none", "@interface\n" + "- (id)init;"); + ASSERT_TRUE((bool)Style); + EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); + Style = getStyle("LLVM", "a.h", "none", "@interface\n" "+ (id)init;"); - EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language); + ASSERT_TRUE((bool)Style); + EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); // No recognizable ObjC. Style = getStyle("LLVM", "a.h", "none", "void f() {}"); - EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language); + ASSERT_TRUE((bool)Style); + EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); } TEST_F(FormatTestObjC, FormatObjCTryCatch) {