Author: nataliakokoromyti Date: 2026-03-14T01:07:57-07:00 New Revision: 7894e636f31a30b303b51292c4bf53931d1e82f7
URL: https://github.com/llvm/llvm-project/commit/7894e636f31a30b303b51292c4bf53931d1e82f7 DIFF: https://github.com/llvm/llvm-project/commit/7894e636f31a30b303b51292c4bf53931d1e82f7.diff LOG: [clang-format] Ignore imports in comments for Java import sorting (#177326) Java source files can contain apparent import statements inside block comments (e.g., showing a code example). These can get mixed up with real import statements when run through clang-format. This patch tracks block comments (/* ... */) so that we skip lines that are inside them. Fixes #176771 --------- Co-authored-by: Natalia Kokoromyti <[email protected]> Co-authored-by: owenca <[email protected]> Added: Modified: clang/lib/Format/Format.cpp clang/unittests/Format/SortImportsTestJava.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 47be33299eadb..b98a9086811cd 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -3825,8 +3825,10 @@ static void sortJavaImports(const FormatStyle &Style, namespace { -const char JavaImportRegexPattern[] = - "^[\t ]*import[\t ]+(static[\t ]*)?([^\t ]*)[\t ]*;"; +constexpr StringRef + JavaImportRegexPattern("^import[\t ]+(static[\t ]*)?([^\t ]*)[\t ]*;"); + +constexpr StringRef JavaPackageRegexPattern("^package[\t ]"); } // anonymous namespace @@ -3835,26 +3837,43 @@ tooling::Replacements sortJavaImports(const FormatStyle &Style, StringRef Code, StringRef FileName, tooling::Replacements &Replaces) { unsigned Prev = 0; - unsigned SearchFrom = 0; + bool HasImport = false; llvm::Regex ImportRegex(JavaImportRegexPattern); + llvm::Regex PackageRegex(JavaPackageRegexPattern); SmallVector<StringRef, 4> Matches; SmallVector<JavaImportDirective, 16> ImportsInBlock; SmallVector<StringRef> AssociatedCommentLines; - bool FormattingOff = false; - - for (;;) { - auto Pos = Code.find('\n', SearchFrom); - StringRef Line = - Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev); + for (bool FormattingOff = false;;) { + auto Pos = Code.find('\n', Prev); + auto GetLine = [&] { + return Code.substr(Prev, + (Pos != StringRef::npos ? Pos : Code.size()) - Prev); + }; + StringRef Line = GetLine(); StringRef Trimmed = Line.trim(); - if (isClangFormatOff(Trimmed)) + if (Trimmed.empty() || PackageRegex.match(Trimmed)) { + // Skip empty line and package statement. + } else if (isClangFormatOff(Trimmed)) { FormattingOff = true; - else if (isClangFormatOn(Trimmed)) + } else if (isClangFormatOn(Trimmed)) { FormattingOff = false; - - if (ImportRegex.match(Line, &Matches)) { + } else if (Trimmed.starts_with("//")) { + // Associating comments within the imports with the nearest import below. + if (HasImport) + AssociatedCommentLines.push_back(Line); + } else if (Trimmed.starts_with("/*")) { + Pos = Code.find("*/", Pos + 2); + if (Pos != StringRef::npos) + Pos = Code.find('\n', Pos + 2); + if (HasImport) { + // Extend `Line` for a multiline comment to include all lines the + // comment spans. + Line = GetLine(); + AssociatedCommentLines.push_back(Line); + } + } else if (ImportRegex.match(Trimmed, &Matches)) { if (FormattingOff) { // If at least one import line has formatting turned off, turn off // formatting entirely. @@ -3867,17 +3886,18 @@ tooling::Replacements sortJavaImports(const FormatStyle &Style, StringRef Code, IsStatic = true; ImportsInBlock.push_back( {Identifier, Line, Prev, AssociatedCommentLines, IsStatic}); + HasImport = true; AssociatedCommentLines.clear(); - } else if (!Trimmed.empty() && !ImportsInBlock.empty()) { - // Associating comments within the imports with the nearest import below - AssociatedCommentLines.push_back(Line); + } else { + // `Trimmed` is neither empty, nor a comment or a package/import + // statement. + break; } - Prev = Pos + 1; if (Pos == StringRef::npos || Pos + 1 == Code.size()) break; - SearchFrom = Pos + 1; + Prev = Pos + 1; } - if (!ImportsInBlock.empty()) + if (HasImport) sortJavaImports(Style, ImportsInBlock, Ranges, FileName, Code, Replaces); return Replaces; } diff --git a/clang/unittests/Format/SortImportsTestJava.cpp b/clang/unittests/Format/SortImportsTestJava.cpp index 26674c75e97b1..4e7111e7e7dff 100644 --- a/clang/unittests/Format/SortImportsTestJava.cpp +++ b/clang/unittests/Format/SortImportsTestJava.cpp @@ -349,6 +349,37 @@ TEST_F(SortImportsTestJava, NoReplacementsForValidImportsWindows) { sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty()); } +TEST_F(SortImportsTestJava, DoNotSortImportsInBlockComment) { + constexpr StringRef Code("/* import org.d;\n" + "import org.c;\n" + "import org.b; */\n" + "import org.a;"); + EXPECT_EQ(Code, sort(Code)); +} + +TEST_F(SortImportsTestJava, StopAtClassDeclaration) { + constexpr StringRef Code("import org.a;\n" + "\n" + "class Foo {\n" + " String code = \"\"\"\n" + " import org.c;\n" + " import org.b;\n" + " \"\"\";\n" + "}"); + EXPECT_EQ(Code, sort(Code)); +} + +TEST_F(SortImportsTestJava, SortImportsAfterPackageStatement) { + EXPECT_EQ("package org.a;\n" + "\n" + "import org.a;\n" + "import org.b;", + sort("package org.a;\n" + "\n" + "import org.b;\n" + "import org.a;")); +} + } // end namespace } // end namespace format } // end namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
