From 31a6506c8e8ca93e70165849570b4e9cbf91ed2e Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Tue, 10 Dec 2013 11:28:13 +0000 Subject: [PATCH] Allow predefined styles to define different options for different languages. Summary: Allow predefined styles to define different options for different languages so that one can run: clang-format -style=google file1.cpp file2.js or use a single .clang-format file with "BasedOnStyle: Google" for both c++ and JS files. Added Google style for JavaScript with "BreakBeforeTernaryOperators" set to false. Reviewers: djasper Reviewed By: djasper CC: cfe-commits, klimek Differential Revision: http://llvm-reviews.chandlerc.com/D2364 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@196909 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Format/Format.h | 16 +++- lib/Format/Format.cpp | 131 +++++++++++++++++------------- unittests/Format/FormatTest.cpp | 109 +++++++++++++++++++------ unittests/Format/FormatTestJS.cpp | 30 ++++--- 4 files changed, 188 insertions(+), 98 deletions(-) diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index 954158b2d5..347e7b7c1c 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -342,6 +342,11 @@ FormatStyle getLLVMStyle(); /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml. FormatStyle getGoogleStyle(); +/// \brief Returns a format style complying with Google's JavaScript style +/// guide: +/// http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml. +FormatStyle getGoogleJSStyle(); + /// \brief Returns a format style complying with Chromium's style guide: /// http://www.chromium.org/developers/coding-style. FormatStyle getChromiumStyle(); @@ -354,15 +359,22 @@ FormatStyle getMozillaStyle(); /// http://www.webkit.org/coding/coding-style.html FormatStyle getWebKitStyle(); -/// \brief Gets a predefined style by name. +/// \brief Gets a predefined style for the specified language by name. /// /// Currently supported names: LLVM, Google, Chromium, Mozilla. Names are /// compared case-insensitively. /// /// Returns \c true if the Style has been set. -bool getPredefinedStyle(StringRef Name, FormatStyle *Style); +bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, + FormatStyle *Style); /// \brief Parse configuration from YAML-formatted text. +/// +/// Style->Language is used to get the base style, if the \c BasedOnStyle +/// option is present. +/// +/// When \c BasedOnStyle is not present, options not present in the YAML +/// document, are retained in \p Style. llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style); /// \brief Gets configuration in a YAML string. diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 91469a66ee..da088bf5cb 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -97,7 +97,10 @@ struct ScalarEnumerationTraits { }; template <> struct MappingTraits { - static void mapping(llvm::yaml::IO &IO, FormatStyle &Style) { + static void mapping(IO &IO, FormatStyle &Style) { + // When reading, read the language first, we need it for getPredefinedStyle. + IO.mapOptional("Language", Style.Language); + if (IO.outputting()) { StringRef StylesArray[] = { "LLVM", "Google", "Chromium", "Mozilla", "WebKit" }; @@ -105,7 +108,7 @@ template <> struct MappingTraits { for (size_t i = 0, e = Styles.size(); i < e; ++i) { StringRef StyleName(Styles[i]); FormatStyle PredefinedStyle; - if (getPredefinedStyle(StyleName, &PredefinedStyle) && + if (getPredefinedStyle(StyleName, Style.Language, &PredefinedStyle) && Style == PredefinedStyle) { IO.mapOptional("# BasedOnStyle", StyleName); break; @@ -115,16 +118,17 @@ template <> struct MappingTraits { StringRef BasedOnStyle; IO.mapOptional("BasedOnStyle", BasedOnStyle); if (!BasedOnStyle.empty()) { - FormatStyle::LanguageKind Language = Style.Language; - if (!getPredefinedStyle(BasedOnStyle, &Style)) { + FormatStyle::LanguageKind OldLanguage = Style.Language; + FormatStyle::LanguageKind Language = + ((FormatStyle *)IO.getContext())->Language; + if (!getPredefinedStyle(BasedOnStyle, Language, &Style)) { IO.setError(Twine("Unknown value for BasedOnStyle: ", BasedOnStyle)); return; } - Style.Language = Language; + Style.Language = OldLanguage; } } - IO.mapOptional("Language", Style.Language); IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset); IO.mapOptional("ConstructorInitializerIndentWidth", Style.ConstructorInitializerIndentWidth); @@ -199,30 +203,28 @@ template <> struct MappingTraits { }; // Allows to read vector while keeping default values. -// Elements will be written or read starting from the 1st element. -// When writing, the 0th element is ignored. -// When reading, keys that are not present in the serialized form will be -// copied from the 0th element of the vector. If the first element had no -// Language specified, it will be treated as the default one for the following -// elements. +// IO.getContext() should contain a pointer to the FormatStyle structure, that +// will be used to get default values for missing keys. +// If the first element has no Language specified, it will be treated as the +// default one for the following elements. template <> struct DocumentListTraits > { - static size_t size(IO &io, std::vector &Seq) { - return Seq.size() - 1; + static size_t size(IO &IO, std::vector &Seq) { + return Seq.size(); } - static FormatStyle &element(IO &io, std::vector &Seq, + static FormatStyle &element(IO &IO, std::vector &Seq, size_t Index) { - if (Index + 2 > Seq.size()) { - assert(Index + 2 == Seq.size() + 1); + if (Index >= Seq.size()) { + assert(Index == Seq.size()); FormatStyle Template; - if (Seq.size() > 1 && Seq[1].Language == FormatStyle::LK_None) { - Template = Seq[1]; - } else { + if (Seq.size() > 0 && Seq[0].Language == FormatStyle::LK_None) { Template = Seq[0]; + } else { + Template = *((const FormatStyle*)IO.getContext()); Template.Language = FormatStyle::LK_None; } - Seq.resize(Index + 2, Template); + Seq.resize(Index + 1, Template); } - return Seq[Index + 1]; + return Seq[Index]; } }; } @@ -336,6 +338,16 @@ FormatStyle getGoogleStyle() { return GoogleStyle; } +FormatStyle getGoogleJSStyle() { + FormatStyle GoogleJSStyle = getGoogleStyle(); + GoogleJSStyle.Language = FormatStyle::LK_JavaScript; + GoogleJSStyle.BreakBeforeTernaryOperators = false; + // FIXME: Currently unimplemented: + // var arr = [1, 2, 3]; // No space after [ or before ]. + // var obj = {a: 1, b: 2, c: 3}; // No space after ':'. + return GoogleJSStyle; +} + FormatStyle getChromiumStyle() { FormatStyle ChromiumStyle = getGoogleStyle(); ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false; @@ -373,56 +385,67 @@ FormatStyle getWebKitStyle() { return Style; } -bool getPredefinedStyle(StringRef Name, FormatStyle *Style) { - if (Name.equals_lower("llvm")) +bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, + FormatStyle *Style) { + if (Name.equals_lower("llvm")) { *Style = getLLVMStyle(); - else if (Name.equals_lower("chromium")) + } else if (Name.equals_lower("chromium")) { *Style = getChromiumStyle(); - else if (Name.equals_lower("mozilla")) + } else if (Name.equals_lower("mozilla")) { *Style = getMozillaStyle(); - else if (Name.equals_lower("google")) - *Style = getGoogleStyle(); - else if (Name.equals_lower("webkit")) + } else if (Name.equals_lower("google")) { + *Style = Language == FormatStyle::LK_JavaScript ? getGoogleJSStyle() + : getGoogleStyle(); + } else if (Name.equals_lower("webkit")) { *Style = getWebKitStyle(); - else + } else { return false; + } + Style->Language = Language; return true; } llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style) { assert(Style); - assert(Style->Language != FormatStyle::LK_None); + FormatStyle::LanguageKind Language = Style->Language; + assert(Language != FormatStyle::LK_None); if (Text.trim().empty()) return llvm::make_error_code(llvm::errc::invalid_argument); std::vector Styles; - // DocumentListTraits> uses 0th element as the default one - // for the fields, keys for which are missing from the configuration. - Styles.push_back(*Style); llvm::yaml::Input Input(Text); + // DocumentListTraits> uses the context to get default + // values for the fields, keys for which are missing from the configuration. + // Mapping also uses the context to get the language to find the correct + // base style. + Input.setContext(Style); Input >> Styles; if (Input.error()) return Input.error(); - for (unsigned i = 1; i < Styles.size(); ++i) { + for (unsigned i = 0; i < Styles.size(); ++i) { // Ensures that only the first configuration can skip the Language option. - if (Styles[i].Language == FormatStyle::LK_None && i != 1) + if (Styles[i].Language == FormatStyle::LK_None && i != 0) return llvm::make_error_code(llvm::errc::invalid_argument); // Ensure that each language is configured at most once. - for (unsigned j = 1; j < i; ++j) { - if (Styles[i].Language == Styles[j].Language) + for (unsigned j = 0; j < i; ++j) { + if (Styles[i].Language == Styles[j].Language) { + DEBUG(llvm::dbgs() + << "Duplicate languages in the config file on positions " << j + << " and " << i << "\n"); return llvm::make_error_code(llvm::errc::invalid_argument); + } } } // Look for a suitable configuration starting from the end, so we can // find the configuration for the specific language first, and the default - // configuration (which can only be at slot 1) after it. - for (unsigned i = Styles.size() - 1; i > 0; --i) { - if (Styles[i].Language == Styles[0].Language || + // configuration (which can only be at slot 0) after it. + for (int i = Styles.size() - 1; i >= 0; --i) { + if (Styles[i].Language == Language || Styles[i].Language == FormatStyle::LK_None) { *Style = Styles[i]; - Style->Language = Styles[0].Language; + Style->Language = Language; return llvm::make_error_code(llvm::errc::success); } } @@ -1667,28 +1690,22 @@ const char *StyleOptionHelpDescription = "parameters, e.g.:\n" " -style=\"{BasedOnStyle: llvm, IndentWidth: 8}\""; -static void fillLanguageByFileName(StringRef FileName, FormatStyle *Style) { - if (FileName.endswith_lower(".c") || FileName.endswith_lower(".h") || - FileName.endswith_lower(".cpp") || FileName.endswith_lower(".hpp") || - FileName.endswith_lower(".cc") || FileName.endswith_lower(".hh") || - FileName.endswith_lower(".cxx") || FileName.endswith_lower(".hxx") || - FileName.endswith_lower(".m") || FileName.endswith_lower(".mm")) { - Style->Language = FormatStyle::LK_Cpp; - } +static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) { if (FileName.endswith_lower(".js")) { - Style->Language = FormatStyle::LK_JavaScript; + return FormatStyle::LK_JavaScript; } + return FormatStyle::LK_Cpp; } FormatStyle getStyle(StringRef StyleName, StringRef FileName, StringRef FallbackStyle) { - FormatStyle Style; - if (!getPredefinedStyle(FallbackStyle, &Style)) { + FormatStyle Style = getLLVMStyle(); + Style.Language = getLanguageByFileName(FileName); + if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { llvm::errs() << "Invalid fallback style \"" << FallbackStyle << "\" using LLVM style\n"; - return getLLVMStyle(); + return Style; } - fillLanguageByFileName(FileName, &Style); if (StyleName.startswith("{")) { // Parse YAML/JSON style from the command line. @@ -1700,13 +1717,13 @@ FormatStyle getStyle(StringRef StyleName, StringRef FileName, } if (!StyleName.equals_lower("file")) { - if (!getPredefinedStyle(StyleName, &Style)) + if (!getPredefinedStyle(StyleName, Style.Language, &Style)) llvm::errs() << "Invalid value for -style, using " << FallbackStyle << " style\n"; - fillLanguageByFileName(FileName, &Style); return Style; } + // Look for .clang-format/_clang-format file in the file's parent directories. SmallString<128> UnsuitableConfigFiles; SmallString<128> Path(FileName); llvm::sys::fs::make_absolute(Path); diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index faaafc2c88..562e67d2a9 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -6950,42 +6950,88 @@ TEST_F(FormatTest, UnderstandsPragmas) { verifyFormat("#pragma omp reduction(+ : var)"); } -bool allStylesEqual(ArrayRef Styles) { - for (size_t i = 1; i < Styles.size(); ++i) - if (!(Styles[0] == Styles[i])) - return false; - return true; -} +#define EXPECT_ALL_STYLES_EQUAL(Styles) \ + for (size_t i = 1; i < Styles.size(); ++i) \ + EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " \ + << Styles.size() \ + << " differs from Style #0" TEST_F(FormatTest, GetsPredefinedStyleByName) { - FormatStyle Styles[3]; + SmallVector Styles; + Styles.resize(3); Styles[0] = getLLVMStyle(); - EXPECT_TRUE(getPredefinedStyle("LLVM", &Styles[1])); - EXPECT_TRUE(getPredefinedStyle("lLvM", &Styles[2])); - EXPECT_TRUE(allStylesEqual(Styles)); + EXPECT_TRUE(getPredefinedStyle("LLVM", FormatStyle::LK_Cpp, &Styles[1])); + EXPECT_TRUE(getPredefinedStyle("lLvM", FormatStyle::LK_Cpp, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); Styles[0] = getGoogleStyle(); - EXPECT_TRUE(getPredefinedStyle("Google", &Styles[1])); - EXPECT_TRUE(getPredefinedStyle("gOOgle", &Styles[2])); - EXPECT_TRUE(allStylesEqual(Styles)); + EXPECT_TRUE(getPredefinedStyle("Google", FormatStyle::LK_Cpp, &Styles[1])); + EXPECT_TRUE(getPredefinedStyle("gOOgle", FormatStyle::LK_Cpp, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); + + Styles[0] = getGoogleJSStyle(); + EXPECT_TRUE( + getPredefinedStyle("Google", FormatStyle::LK_JavaScript, &Styles[1])); + EXPECT_TRUE( + getPredefinedStyle("gOOgle", FormatStyle::LK_JavaScript, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); Styles[0] = getChromiumStyle(); - EXPECT_TRUE(getPredefinedStyle("Chromium", &Styles[1])); - EXPECT_TRUE(getPredefinedStyle("cHRoMiUM", &Styles[2])); - EXPECT_TRUE(allStylesEqual(Styles)); + EXPECT_TRUE(getPredefinedStyle("Chromium", FormatStyle::LK_Cpp, &Styles[1])); + EXPECT_TRUE(getPredefinedStyle("cHRoMiUM", FormatStyle::LK_Cpp, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); Styles[0] = getMozillaStyle(); - EXPECT_TRUE(getPredefinedStyle("Mozilla", &Styles[1])); - EXPECT_TRUE(getPredefinedStyle("moZILla", &Styles[2])); - EXPECT_TRUE(allStylesEqual(Styles)); + EXPECT_TRUE(getPredefinedStyle("Mozilla", FormatStyle::LK_Cpp, &Styles[1])); + EXPECT_TRUE(getPredefinedStyle("moZILla", FormatStyle::LK_Cpp, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); Styles[0] = getWebKitStyle(); - EXPECT_TRUE(getPredefinedStyle("WebKit", &Styles[1])); - EXPECT_TRUE(getPredefinedStyle("wEbKit", &Styles[2])); - EXPECT_TRUE(allStylesEqual(Styles)); + EXPECT_TRUE(getPredefinedStyle("WebKit", FormatStyle::LK_Cpp, &Styles[1])); + EXPECT_TRUE(getPredefinedStyle("wEbKit", FormatStyle::LK_Cpp, &Styles[2])); + EXPECT_ALL_STYLES_EQUAL(Styles); + + EXPECT_FALSE(getPredefinedStyle("qwerty", FormatStyle::LK_Cpp, &Styles[0])); +} - EXPECT_FALSE(getPredefinedStyle("qwerty", &Styles[0])); +TEST_F(FormatTest, GetsCorrectBasedOnStyle) { + SmallVector Styles; + Styles.resize(2); + + Styles[0] = getGoogleStyle(); + Styles[1] = getLLVMStyle(); + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1]).value()); + EXPECT_ALL_STYLES_EQUAL(Styles); + + Styles.resize(5); + Styles[0] = getGoogleJSStyle(); + Styles[1] = getLLVMStyle(); + Styles[1].Language = FormatStyle::LK_JavaScript; + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1]).value()); + + Styles[2] = getLLVMStyle(); + Styles[2].Language = FormatStyle::LK_JavaScript; + EXPECT_EQ(0, parseConfiguration("Language: JavaScript\n" + "BasedOnStyle: Google", + &Styles[2]).value()); + + Styles[3] = getLLVMStyle(); + Styles[3].Language = FormatStyle::LK_JavaScript; + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google\n" + "Language: JavaScript", + &Styles[3]).value()); + + Styles[4] = getLLVMStyle(); + Styles[4].Language = FormatStyle::LK_JavaScript; + EXPECT_EQ(0, parseConfiguration("---\n" + "BasedOnStyle: LLVM\n" + "IndentWidth: 123\n" + "---\n" + "BasedOnStyle: Google\n" + "Language: JavaScript", + &Styles[4]).value()); + EXPECT_ALL_STYLES_EQUAL(Styles); } #define CHECK_PARSE(TEXT, FIELD, VALUE) \ @@ -7192,6 +7238,23 @@ TEST_F(FormatTest, ParsesConfigurationWithLanguages) { EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language); } +TEST_F(FormatTest, UsesLanguageForBasedOnStyle) { + FormatStyle Style = {}; + Style.Language = FormatStyle::LK_JavaScript; + Style.BreakBeforeTernaryOperators = true; + CHECK_PARSE("BasedOnStyle: Google", BreakBeforeTernaryOperators, false); + Style.BreakBeforeTernaryOperators = true; + CHECK_PARSE("---\n" + "BasedOnStyle: Google\n" + "---\n" + "Language: JavaScript\n" + "IndentWidth: 76\n" + "...\n", + BreakBeforeTernaryOperators, false); + EXPECT_EQ(76u, Style.IndentWidth); + EXPECT_EQ(FormatStyle::LK_JavaScript, Style.Language); +} + #undef CHECK_PARSE #undef CHECK_PARSE_BOOL diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp index 1f8f139574..0d242a7bc9 100644 --- a/unittests/Format/FormatTestJS.cpp +++ b/unittests/Format/FormatTestJS.cpp @@ -33,24 +33,18 @@ protected: } static std::string format(llvm::StringRef Code, - const FormatStyle &Style = getJSStyle()) { + const FormatStyle &Style = getGoogleJSStyle()) { return format(Code, 0, Code.size(), Style); } - static FormatStyle getJSStyle() { - FormatStyle Style = getLLVMStyle(); - Style.Language = FormatStyle::LK_JavaScript; - return Style; - } - - static FormatStyle getJSStyleWithColumns(unsigned ColumnLimit) { - FormatStyle Style = getJSStyle(); + static FormatStyle getGoogleJSStyleWithColumns(unsigned ColumnLimit) { + FormatStyle Style = getGoogleJSStyle(); Style.ColumnLimit = ColumnLimit; return Style; } static void verifyFormat(llvm::StringRef Code, - const FormatStyle &Style = getJSStyle()) { + const FormatStyle &Style = getGoogleJSStyle()) { EXPECT_EQ(Code.str(), format(test::messUp(Code), Style)); } }; @@ -60,26 +54,30 @@ TEST_F(FormatTestJS, UnderstandsJavaScriptOperators) { verifyFormat("a != = b;"); verifyFormat("a === b;"); - verifyFormat("aaaaaaa ===\n b;", getJSStyleWithColumns(10)); + verifyFormat("aaaaaaa ===\n b;", getGoogleJSStyleWithColumns(10)); verifyFormat("a !== b;"); - verifyFormat("aaaaaaa !==\n b;", getJSStyleWithColumns(10)); + verifyFormat("aaaaaaa !==\n b;", getGoogleJSStyleWithColumns(10)); verifyFormat("if (a + b + c +\n" " d !==\n" " e + f + g)\n" " q();", - getJSStyleWithColumns(20)); + getGoogleJSStyleWithColumns(20)); verifyFormat("a >> >= b;"); verifyFormat("a >>> b;"); - verifyFormat("aaaaaaa >>>\n b;", getJSStyleWithColumns(10)); + verifyFormat("aaaaaaa >>>\n b;", getGoogleJSStyleWithColumns(10)); verifyFormat("a >>>= b;"); - verifyFormat("aaaaaaa >>>=\n b;", getJSStyleWithColumns(10)); + verifyFormat("aaaaaaa >>>=\n b;", getGoogleJSStyleWithColumns(10)); verifyFormat("if (a + b + c +\n" " d >>>\n" " e + f + g)\n" " q();", - getJSStyleWithColumns(20)); + getGoogleJSStyleWithColumns(20)); + verifyFormat("var x = aaaaaaaaaa ?\n" + " bbbbbb :\n" + " ccc;", + getGoogleJSStyleWithColumns(20)); } } // end namespace tooling -- 2.40.0