r292174 - clang-format: Make GetStyle return Expected<FormatStyle> instead of FormatStyle
Antonio Maiorano via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 16 16:12:27 PST 2017
Author: amaiorano
Date: Mon Jan 16 18:12:27 2017
New Revision: 292174
URL: http://llvm.org/viewvc/llvm-project?rev=292174&view=rev
Log:
clang-format: Make GetStyle return Expected<FormatStyle> instead of FormatStyle
Change the contract of GetStyle so that it returns an error when an error occurs
(i.e. when it writes to stderr), and only returns the fallback style when it
can't find a configuration file.
Differential Revision: https://reviews.llvm.org/D28081
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Tooling/Refactoring.cpp
cfe/trunk/test/Format/style-on-command-line.cpp
cfe/trunk/tools/clang-format/ClangFormat.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Mon Jan 16 18:12:27 2017
@@ -853,17 +853,19 @@ extern const char *StyleOptionHelpDescri
/// \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) {
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Jan 16 18:12:27 2017
@@ -421,6 +421,11 @@ std::error_code make_error_code(ParseErr
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 getLang
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
(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
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
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;
}
Modified: cfe/trunk/lib/Tooling/Refactoring.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring.cpp Mon Jan 16 18:12:27 2017
@@ -68,8 +68,8 @@ int RefactoringTool::saveRewrittenFiles(
}
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;
Modified: cfe/trunk/test/Format/style-on-command-line.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/style-on-command-line.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/test/Format/style-on-command-line.cpp (original)
+++ cfe/trunk/test/Format/style-on-command-line.cpp Mon Jan 16 18:12:27 2017
@@ -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;$}}
Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Mon Jan 16 18:12:27 2017
@@ -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;
}
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 16 18:12:27 2017
@@ -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) {
Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=292174&r1=292173&r2=292174&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Mon Jan 16 18:12:27 2017
@@ -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) {
More information about the cfe-commits
mailing list