llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-format Author: Bugale Bugalit (bugale) <details> <summary>Changes</summary> BEEP BOOP! I am Copilot using Bugale's account: *Per the [LLVM AI Tool Use Policy](https://llvm.org/docs/AIToolPolicy.html): this PR was drafted with an AI assistant (GitHub Copilot, Claude Fable 5) operated by @<!-- -->bugale, who reviewed every line, validated the change locally, and is accountable for it and for addressing review feedback.* Fixes #<!-- -->216988. When set, the `// clang-format off` / `// clang-format on` comments (including their `/* clang-format off */` and `// clang-format off: reason` forms) lose their special meaning: the code between them is formatted like any other code, include sorting is not disabled by them, and the marker comments themselves are treated as ordinary comments. Motivation: a CI check that enforces clang-format on changed lines currently has no way to prevent contributors (or AI code assistants, which increasingly do this) from silencing it by wrapping unformatted code in `// clang-format off` - the markers are hardcoded and no configuration can disable them. With this option, the enforcing check can run with `IgnoreFormatOffComments: true` (e.g. via an inline `--style` override), while developers and editors keep the default `false`, so interactive use of the markers is unaffected; only the enforcing environment ignores them. Markers matched by `OneLineFormatOffRegex` remain in effect: that option is already explicit opt-in configuration under the control of whoever assembles the effective style (an enforcing check that overrides the style can pin it to `""`), unlike the built-in markers, which nothing could disable before this change. The `.clang-format-ignore` file is intentionally out of scope: it is tool-level file discovery rather than a style concern. Implementation: new `isClangFormatOn/Off(StringRef, const FormatStyle &)` overloads return `false` under the option; all in-tree call sites are migrated (token lexing, C++ include sorting, Java and JavaScript import sorting, definition block separator, comment alignment in TokenAnnotator, and the integer-literal separator and numeric-literal case fixers). The single-argument overloads are kept unchanged. `ClangFormatStyleOptions.md` was regenerated with `dump_format_style.py`. All FormatTests pass locally (1274 tests). --- Full diff: https://github.com/llvm/llvm-project/pull/216986.diff 13 Files Affected: - (modified) clang/docs/ClangFormatStyleOptions.md (+14) - (modified) clang/docs/ReleaseNotes.md (+4) - (modified) clang/include/clang/Format/Format.h (+17) - (modified) clang/lib/Format/DefinitionBlockSeparator.cpp (+2-2) - (modified) clang/lib/Format/Format.cpp (+14-4) - (modified) clang/lib/Format/FormatTokenLexer.cpp (+2-2) - (modified) clang/lib/Format/IntegerLiteralSeparatorFixer.cpp (+2-2) - (modified) clang/lib/Format/NumericLiteralCaseFixer.cpp (+2-2) - (modified) clang/lib/Format/SortJavaScriptImports.cpp (+4-3) - (modified) clang/lib/Format/TokenAnnotator.cpp (+1-1) - (modified) clang/unittests/Format/ConfigParseTest.cpp (+1) - (modified) clang/unittests/Format/FormatTest.cpp (+53) - (modified) clang/unittests/Format/SortIncludesTest.cpp (+21) ``````````diff diff --git a/clang/docs/ClangFormatStyleOptions.md b/clang/docs/ClangFormatStyleOptions.md index 8230e6f5139e7..4c6b353544421 100644 --- a/clang/docs/ClangFormatStyleOptions.md +++ b/clang/docs/ClangFormatStyleOptions.md @@ -4581,6 +4581,20 @@ the configuration (without a prefix: `Auto`). For example: [KJ_IF_MAYBE](https://github.com/capnproto/capnproto/blob/master/kjdoc/tour.md#maybes) +(ignoreformatoffcomments)= + +**IgnoreFormatOffComments** (`Boolean`) {versionbadge}`clang-format 23` {ref}`¶ <IgnoreFormatOffComments>` + +: If `true`, the `// clang-format off` and `// clang-format on` comments + have no effect: the code between them is formatted like any other, and + include sorting is not disabled by them. This lets an environment that + enforces a format check make sure the check cannot be suppressed from + inside a file. + + :::{note} + Markers matched by `OneLineFormatOffRegex` are still in effect. + ::: + (includeblocks)= **IncludeBlocks** (`IncludeBlocksStyle`) {versionbadge}`clang-format 6` {ref}`¶ <IncludeBlocks>` diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index f5f9958543e34..e9f201b1ba8e5 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -600,6 +600,10 @@ features cannot lower the translation-unit ABI level; `thread_local`, `extern`, `mutable`, `signed`, `unsigned`, `long`, `short`, and `explicit` declaration specifiers. +- Add `IgnoreFormatOffComments` option to strip `// clang-format off` and + `// clang-format on` comments of their special meaning, so that formatting + cannot be suppressed from inside a file. + ### libclang - visit identifier initializers in lambda capture as VarDecl instead of VariableRef. Warning: this changes behaviour. diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 49e4666ae5e99..3399ae09b614f 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3161,6 +3161,17 @@ struct FormatStyle { /// \version 13 std::vector<std::string> IfMacros; + /// If `true`, the `// clang-format off` and `// clang-format on` comments + /// have no effect: the code between them is formatted like any other, and + /// include sorting is not disabled by them. This lets an environment that + /// enforces a format check make sure the check cannot be suppressed from + /// inside a file. + /// \note + /// Markers matched by `OneLineFormatOffRegex` are still in effect. + /// \endnote + /// \version 23 + bool IgnoreFormatOffComments; + /// Specify whether access modifiers should have their own indentation level. /// /// When `false`, access modifiers are indented (or outdented) relative to @@ -6191,6 +6202,7 @@ struct FormatStyle { R.ExperimentalAutoDetectBinPacking && FixNamespaceComments == R.FixNamespaceComments && ForEachMacros == R.ForEachMacros && + IgnoreFormatOffComments == R.IgnoreFormatOffComments && IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks && IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories && IncludeStyle.IncludeIsMainRegex == @@ -6625,6 +6637,11 @@ inline StringRef getLanguageName(FormatStyle::LanguageKind Language) { bool isClangFormatOn(StringRef Comment); bool isClangFormatOff(StringRef Comment); +// Like the overloads above, but return false when the style ignores the +// clang-format on/off comments. +bool isClangFormatOn(StringRef Comment, const FormatStyle &Style); +bool isClangFormatOff(StringRef Comment, const FormatStyle &Style); + } // end namespace format } // end namespace clang diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp b/clang/lib/Format/DefinitionBlockSeparator.cpp index 6b52b1fc0deff..5edf25bda9c3d 100644 --- a/clang/lib/Format/DefinitionBlockSeparator.cpp +++ b/clang/lib/Format/DefinitionBlockSeparator.cpp @@ -90,7 +90,7 @@ void DefinitionBlockSeparator::separateBlocks( // Lines should not be added in the disabled region. if (TargetToken->is(tok::comment) && - isClangFormatOn(TargetToken->TokenText)) { + isClangFormatOn(TargetToken->TokenText, Style)) { return; } // Do not handle EOF newlines. @@ -155,7 +155,7 @@ void DefinitionBlockSeparator::separateBlocks( OperateIndex + 1 < Lines.size() ? Lines[OperateIndex + 1] : nullptr; if (const auto *Tok = OperateLine->First; - Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) { + Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText, Style)) { const bool IsEndComment = Tok->NewlinesBefore == 1 && NextLine && NextLine->First->NewlinesBefore > 1; if (!IsEndComment) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 2b6e65efbf026..5ef62c3ee741b 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1385,6 +1385,7 @@ template <> struct MappingTraits<FormatStyle> { IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments); IO.mapOptional("ForEachMacros", Style.ForEachMacros); IO.mapOptional("IfMacros", Style.IfMacros); + IO.mapOptional("IgnoreFormatOffComments", Style.IgnoreFormatOffComments); IO.mapOptional("IncludeBlocks", Style.IncludeStyle.IncludeBlocks); IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories); IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex); @@ -1951,6 +1952,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.ForEachMacros.push_back("Q_FOREACH"); LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH"); LLVMStyle.IfMacros.push_back("KJ_IF_MAYBE"); + LLVMStyle.IgnoreFormatOffComments = false; LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve; LLVMStyle.IncludeStyle.IncludeCategories = { {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, @@ -3831,9 +3833,9 @@ tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code, bool IsBlockComment = false; - if (isClangFormatOff(Trimmed)) { + if (isClangFormatOff(Trimmed, Style)) { FormattingOff = true; - } else if (isClangFormatOn(Trimmed)) { + } else if (isClangFormatOn(Trimmed, Style)) { FormattingOff = false; } else if (Trimmed.starts_with("/*")) { IsBlockComment = true; @@ -4023,9 +4025,9 @@ tooling::Replacements sortJavaImports(const FormatStyle &Style, StringRef Code, StringRef Trimmed = Line.trim(); if (Trimmed.empty() || PackageRegex.match(Trimmed)) { // Skip empty line and package statement. - } else if (isClangFormatOff(Trimmed)) { + } else if (isClangFormatOff(Trimmed, Style)) { FormattingOff = true; - } else if (isClangFormatOn(Trimmed)) { + } else if (isClangFormatOn(Trimmed, Style)) { FormattingOff = false; } else if (Trimmed.starts_with("//")) { // Associating comments within the imports with the nearest import below. @@ -4917,5 +4919,13 @@ bool isClangFormatOff(StringRef Comment) { return isClangFormatOnOff(Comment, /*On=*/false); } +bool isClangFormatOn(StringRef Comment, const FormatStyle &Style) { + return !Style.IgnoreFormatOffComments && isClangFormatOn(Comment); +} + +bool isClangFormatOff(StringRef Comment, const FormatStyle &Style) { + return !Style.IgnoreFormatOffComments && isClangFormatOff(Comment); +} + } // namespace format } // namespace clang diff --git a/clang/lib/Format/FormatTokenLexer.cpp b/clang/lib/Format/FormatTokenLexer.cpp index 2a8bef21ad414..8d0dde82105f0 100644 --- a/clang/lib/Format/FormatTokenLexer.cpp +++ b/clang/lib/Format/FormatTokenLexer.cpp @@ -1633,12 +1633,12 @@ void FormatTokenLexer::readRawToken(FormatToken &Tok) { if ((Style.isJavaScript() || Style.isProto()) && Tok.is(tok::char_constant)) Tok.Tok.setKind(tok::string_literal); - if (Tok.is(tok::comment) && isClangFormatOn(Tok.TokenText)) + if (Tok.is(tok::comment) && isClangFormatOn(Tok.TokenText, Style)) FormattingDisabled = false; Tok.Finalized = FormattingDisabled; - if (Tok.is(tok::comment) && isClangFormatOff(Tok.TokenText)) + if (Tok.is(tok::comment) && isClangFormatOff(Tok.TokenText, Style)) FormattingDisabled = true; } diff --git a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp index eea9fcc56af63..478ed65040eff 100644 --- a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp +++ b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp @@ -108,9 +108,9 @@ IntegerLiteralSeparatorFixer::process(const Environment &Env, auto Location = Tok.getLocation(); auto Text = StringRef(SourceMgr.getCharacterData(Location), Length); if (Tok.is(tok::comment)) { - if (isClangFormatOff(Text)) + if (isClangFormatOff(Text, Style)) Skip = true; - else if (isClangFormatOn(Text)) + else if (isClangFormatOn(Text, Style)) Skip = false; continue; } diff --git a/clang/lib/Format/NumericLiteralCaseFixer.cpp b/clang/lib/Format/NumericLiteralCaseFixer.cpp index b58b3c7ee0189..2f97190d5f88c 100644 --- a/clang/lib/Format/NumericLiteralCaseFixer.cpp +++ b/clang/lib/Format/NumericLiteralCaseFixer.cpp @@ -150,9 +150,9 @@ NumericLiteralCaseFixer::process(const Environment &Env, auto Location = Tok.getLocation(); auto Text = StringRef(SourceMgr.getCharacterData(Location), Length); if (Tok.is(tok::comment)) { - if (isClangFormatOff(Text)) + if (isClangFormatOff(Text, Style)) Skip = true; - else if (isClangFormatOn(Text)) + else if (isClangFormatOn(Text, Style)) Skip = false; continue; } diff --git a/clang/lib/Format/SortJavaScriptImports.cpp b/clang/lib/Format/SortJavaScriptImports.cpp index a403a4fed664c..bc54f207eab32 100644 --- a/clang/lib/Format/SortJavaScriptImports.cpp +++ b/clang/lib/Format/SortJavaScriptImports.cpp @@ -192,7 +192,8 @@ class JavaScriptImportSorter : public TokenAnalyzer { // Separate references from the main code body of the file. if (FirstNonImportLine && FirstNonImportLine->First->NewlinesBefore < 2 && !(FirstNonImportLine->First->is(tok::comment) && - isClangFormatOn(FirstNonImportLine->First->TokenText.trim()))) { + isClangFormatOn(FirstNonImportLine->First->TokenText.trim(), + Style))) { ReferencesText += "\n"; } @@ -372,9 +373,9 @@ class JavaScriptImportSorter : public TokenAnalyzer { // This is tracked in FormattingOff here and on JsModuleReference. while (Current && Current->is(tok::comment)) { StringRef CommentText = Current->TokenText.trim(); - if (isClangFormatOff(CommentText)) { + if (isClangFormatOff(CommentText, Style)) { FormattingOff = true; - } else if (isClangFormatOn(CommentText)) { + } else if (isClangFormatOn(CommentText, Style)) { FormattingOff = false; // Special case: consider a trailing "clang-format on" line to be part // of the module reference, so that it gets moved around together with diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index b6c33279b0aca..830c4112f0fc7 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3671,7 +3671,7 @@ void TokenAnnotator::setCommentLineLevels( // it, that's probably intentional and we should keep it. if (const auto Column = Line->First->OriginalColumn; NextNonCommentLine && NextNonCommentLine->First->NewlinesBefore < 2 && - Line->isComment() && !isClangFormatOff(Line->First->TokenText) && + Line->isComment() && !isClangFormatOff(Line->First->TokenText, Style) && NextNonCommentLine->First->OriginalColumn == Column) { const bool PPDirectiveOrImportStmt = NextNonCommentLine->Type == LT_PreprocessorDirective || diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 9350ba7eb3de4..89e6c0de5aae5 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -189,6 +189,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(DerivePointerAlignment); CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding"); CHECK_PARSE_BOOL(DisableFormat); + CHECK_PARSE_BOOL(IgnoreFormatOffComments); CHECK_PARSE_BOOL(IndentAccessModifiers); CHECK_PARSE_BOOL(IndentCaseBlocks); CHECK_PARSE_BOOL(IndentCaseLabels); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 6f604167f785c..7e142542cc19a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -22422,6 +22422,59 @@ TEST_F(FormatTest, DisableRegions) { "// clang-format on"); } +TEST_F(FormatTest, IgnoreFormatOffComments) { + auto Style = getLLVMStyle(); + Style.IgnoreFormatOffComments = true; + + verifyFormat("int i;\n" + "// clang-format off\n" + "int j;\n" + "// clang-format on\n" + "int k;", + " int i;\n" + " // clang-format off\n" + " int j;\n" + " // clang-format on\n" + " int k;", + Style); + + verifyFormat("int i;\n" + "/* clang-format off */\n" + "int j;\n" + "/* clang-format on */\n" + "int k;", + " int i;\n" + " /* clang-format off */\n" + " int j;\n" + " /* clang-format on */\n" + " int k;", + Style); + + verifyFormat("int *i;\n" + "// clang-format off: reason\n" + "int *j;\n" + "// clang-format on: reason\n" + "int *k;", + "int* i;\n" + "// clang-format off: reason\n" + "int* j;\n" + "// clang-format on: reason\n" + "int* k;", + Style); + + // Markers matched by OneLineFormatOffRegex are still in effect. + Style.OneLineFormatOffRegex = "^// NOLINT$"; + verifyFormat("int i;\n" + "// NOLINT\n" + "int j ;\n" + "int k;", + "int i ;\n" + "// NOLINT\n" + "int j ;\n" + "int k ;", + Style); +} + TEST_F(FormatTest, OneLineFormatOffRegex) { auto Style = getLLVMStyle(); Style.OneLineFormatOffRegex = "// format off$"; diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp index a6e9e18496f8d..17f9c2910418c 100644 --- a/clang/unittests/Format/SortIncludesTest.cpp +++ b/clang/unittests/Format/SortIncludesTest.cpp @@ -283,6 +283,27 @@ TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) { "input.h", 2)); } +TEST_F(SortIncludesTest, IgnoreFormatOffComments) { + FmtStyle.IgnoreFormatOffComments = true; + verifyFormat("#include <a>\n" + "#include <b>\n" + "#include <c>\n" + "// clang-format off\n" + "#include <a>\n" + "#include <b>\n" + "#include <c>\n" + "// clang-format on", + sort("#include <b>\n" + "#include <a>\n" + "#include <c>\n" + "// clang-format off\n" + "#include <b>\n" + "#include <a>\n" + "#include <c>\n" + "// clang-format on", + "input.h", 2)); +} + TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) { FmtStyle.SortIncludes = {}; verifyFormat("#include \"a.h\"\n" `````````` </details> https://github.com/llvm/llvm-project/pull/216986 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
