Index: include/clang/Format/Format.h =================================================================== --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -861,9 +861,10 @@ /// /// \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); +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()); } +template llvm::Error make_string_error(ArgTs &&... Args) { + return llvm::make_error(std::forward(Args)..., + llvm::inconvertibleErrorCode()); +} + const char *ParseErrorCategory::name() const noexcept { return "clang-format.parse_error"; } @@ -1876,50 +1881,50 @@ 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 FallbackStyleName, + StringRef Code, vfs::FileSystem *FS) { if (!FS) { FS = vfs::getRealFileSystem().get(); } - FormatStyle Style = getLLVMStyle(); - Style.Language = getLanguageByFileName(FileName); + FormatStyle::LanguageKind Language = getLanguageByFileName(FileName); // This is a very crude detection of whether a header contains ObjC code that // should be improved over time and probably be done on tokens, not one the // bare content of the file. - if (Style.Language == FormatStyle::LK_Cpp && FileName.endswith(".h") && + if (Language == FormatStyle::LK_Cpp && FileName.endswith(".h") && (Code.contains("\n- (") || Code.contains("\n+ ("))) - Style.Language = FormatStyle::LK_ObjC; + Language = FormatStyle::LK_ObjC; - if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { - llvm::errs() << "Invalid fallback style \"" << FallbackStyle - << "\" using LLVM style\n"; - return Style; + FormatStyle FallbackStyle = getNoStyle(); + if (!getPredefinedStyle(FallbackStyleName, Language, &FallbackStyle)) { + return make_string_error("Invalid fallback style \"" + + FallbackStyleName.str()); } if (StyleName.startswith("{")) { // Parse YAML/JSON style from the command line. + FormatStyle Style; if (std::error_code ec = parseConfiguration(StyleName, &Style)) { - llvm::errs() << "Error parsing -style: " << ec.message() << ", using " - << FallbackStyle << " style\n"; + return make_string_error("Error parsing -style: " + ec.message()); } + Style.Language = Language; return Style; } if (!StyleName.equals_lower("file")) { + FormatStyle Style; if (!getPredefinedStyle(StyleName, Style.Language, &Style)) - llvm::errs() << "Invalid value for -style, using " << FallbackStyle - << " style\n"; - return Style; + return make_string_error("Invalid value for -style"); + Style.Language = Language; + 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; + return make_string_error(EC.message()); } for (StringRef Directory = Path; !Directory.empty(); @@ -1937,25 +1942,25 @@ 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()); } + FormatStyle Style; if (std::error_code ec = parseConfiguration(Text.get()->getBuffer(), &Style)) { if (ec == ParseError::Unsuitable) { @@ -1964,20 +1969,25 @@ 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; + Style.Language = Language; + return Style; } } if (!UnsuitableConfigFiles.empty()) { - llvm::errs() << "Configuration file(s) do(es) not support " - << getLanguageName(Style.Language) << ": " - << UnsuitableConfigFiles << "\n"; + // TODO: If we find an unsuitable file for this language, are we expected to + // use the fallback style? + // If so, can't return this error here... + return make_string_error("Configuration file(s) do(es) not support " + + getLanguageName(Language) + ": " + + UnsuitableConfigFiles); } - return Style; + + // No config file found, return the fallback style, which may be no style + return FallbackStyle; } } // namespace format Index: lib/Tooling/Refactoring.cpp =================================================================== --- lib/Tooling/Refactoring.cpp +++ lib/Tooling/Refactoring.cpp @@ -83,7 +83,13 @@ 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,41 @@ 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 fail to open + // TODO: not sure how to test this + + // Test 8: 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 Style8 = getStyle("file", "/d/.clang-format", "LLVM", "", &FS); + ASSERT_FALSE((bool)Style8); + auto ErrorMsg8 = llvm::toString(Style8.takeError()); + ASSERT_GT(ErrorMsg8.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); }