amaiorano updated this revision to Diff 84538.
amaiorano added a comment.
Rebased changes on latest, no functional change in this diff.
https://reviews.llvm.org/D28081
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Tooling/Refactoring.cpp
test/Format/style-on-command-line.cpp
tools/clang-format/ClangFormat.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===================================================================
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -68,17 +68,21 @@
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) {
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10967,22 +10967,51 @@
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(
FS.addFile("/c/.clang-format", 0,
llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
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) {
Index: tools/clang-format/ClangFormat.cpp
===================================================================
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -249,12 +249,17 @@
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 @@
// 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 @@
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;
}
Index: test/Format/style-on-command-line.cpp
===================================================================
--- test/Format/style-on-command-line.cpp
+++ 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 @@
// 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;$}}
Index: lib/Tooling/Refactoring.cpp
===================================================================
--- lib/Tooling/Refactoring.cpp
+++ lib/Tooling/Refactoring.cpp
@@ -68,8 +68,8 @@
}
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 @@
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;
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -421,6 +421,11 @@
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 @@
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 @@
(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,46 +1941,42 @@
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) {
if (!UnsuitableConfigFiles.empty())
UnsuitableConfigFiles.append(", ");
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;
}
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<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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
