Index: include/clang/Format/Format.h =================================================================== --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -853,17 +853,19 @@ /// \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 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) { Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -421,6 +421,11 @@ return std::error_code(static_cast(e), getParseCategory()); } +llvm::Error make_string_error(const llvm::Twine &Message) { + return llvm::make_error(Message, + llvm::inconvertibleErrorCode()); +} + const char *ParseErrorCategory::name() const noexcept { return "clang-format.parse_error"; } @@ -1876,9 +1881,9 @@ return FormatStyle::LK_Cpp; } -FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle, StringRef Code, - vfs::FileSystem *FS) { +llvm::Expected getStyle(StringRef StyleName, StringRef FileName, + StringRef FallbackStyle, StringRef Code, + vfs::FileSystem *FS) { if (!FS) { FS = vfs::getRealFileSystem().get(); } @@ -1892,35 +1897,30 @@ (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", this effectively disables + // replacements. Fix this by setting a separate FormatStyle variable and + // returning it when we mean to return the fallback style explicitly. + 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)) { @@ -1937,24 +1937,23 @@ 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> Text = FS->getBufferForFile(ConfigFile.str()); if (std::error_code EC = Text.getError()) { - llvm::errs() << EC.message() << "\n"; - break; + return make_string_error(EC.message()); } if (std::error_code ec = parseConfiguration(Text.get()->getBuffer(), &Style)) { @@ -1964,18 +1963,17 @@ 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"; + return make_string_error("Configuration file(s) do(es) not support " + + getLanguageName(Style.Language) + ": " + + UnsuitableConfigFiles); } return Style; } Index: lib/Tooling/Refactoring.cpp =================================================================== --- lib/Tooling/Refactoring.cpp +++ lib/Tooling/Refactoring.cpp @@ -68,8 +68,8 @@ } bool formatAndApplyAllReplacements( - const std::map &FileToReplaces, Rewriter &Rewrite, - StringRef Style) { + const std::map &FileToReplaces, + Rewriter &Rewrite, StringRef Style) { SourceManager &SM = Rewrite.getSourceMgr(); FileManager &Files = SM.getFileManager(); @@ -83,7 +83,14 @@ FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User); StringRef Code = SM.getBufferData(ID); - format::FormatStyle CurStyle = format::getStyle(Style, FilePath, "LLVM"); + llvm::Expected FormatStyleOrError = + format::getStyle(Style, FilePath, "LLVM"); + if (!FormatStyleOrError) { + llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; + return false; + } + const format::FormatStyle CurStyle = *FormatStyleOrError; + auto NewReplacements = format::formatReplacements(Code, CurReplaces, CurStyle); if (!NewReplacements) { Index: tools/clang-format/ClangFormat.cpp =================================================================== --- tools/clang-format/ClangFormat.cpp +++ tools/clang-format/ClangFormat.cpp @@ -249,8 +249,14 @@ if (fillRanges(Code.get(), Ranges)) return true; StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName; - FormatStyle FormatStyle = + + llvm::Expected FormatStyleOrError = getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer()); + if (!FormatStyleOrError) { + llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; + return true; + } + FormatStyle FormatStyle = *FormatStyleOrError; if (SortIncludes.getNumOccurrences() != 0) FormatStyle.SortIncludes = SortIncludes; unsigned CursorPosition = Cursor; @@ -334,10 +340,16 @@ cl::PrintHelpMessage(); if (DumpConfig) { - std::string Config = - clang::format::configurationAsText(clang::format::getStyle( + llvm::Expected FormatStyleOrError = + clang::format::getStyle( Style, FileNames.empty() ? AssumeFileName : FileNames[0], - FallbackStyle)); + FallbackStyle); + if (!FormatStyleOrError) { + llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; + return 1; + } + std::string Config = + clang::format::configurationAsText(*FormatStyleOrError); outs() << Config << "\n"; return 0; } Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -10945,13 +10945,15 @@ 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( @@ -10960,7 +10962,38 @@ 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); + auto ErrorMsg4 = llvm::toString(Style4.takeError()); + ASSERT_GT(ErrorMsg4.length(), 0); + + // Test 5: error on invalid yaml on command line + auto Style5 = getStyle("{invalid_key=invalid_value}", "a.h", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style5); + auto ErrorMsg5 = llvm::toString(Style5.takeError()); + ASSERT_GT(ErrorMsg5.length(), 0); + + // Test 6: error on invalid style + auto Style6 = getStyle("KungFu", "a.h", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style6); + auto ErrorMsg6 = llvm::toString(Style6.takeError()); + ASSERT_GT(ErrorMsg6.length(), 0); + + // 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); + auto ErrorMsg7 = llvm::toString(Style7.takeError()); + ASSERT_GT(ErrorMsg7.length(), 0); } TEST_F(ReplacementTest, FormatCodeAfterReplacements) { Index: unittests/Format/FormatTestObjC.cpp =================================================================== --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -69,15 +69,15 @@ }; TEST_F(FormatTestObjC, DetectsObjCInHeaders) { - Style = getStyle("LLVM", "a.h", "none", "@interface\n" + Style = *getStyle("LLVM", "a.h", "none", "@interface\n" "- (id)init;"); EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language); - Style = getStyle("LLVM", "a.h", "none", "@interface\n" + Style = *getStyle("LLVM", "a.h", "none", "@interface\n" "+ (id)init;"); EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language); // No recognizable ObjC. - Style = getStyle("LLVM", "a.h", "none", "void f() {}"); + Style = *getStyle("LLVM", "a.h", "none", "void f() {}"); EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language); }