[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: jonathanmeier, alexfh.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre, ormris.
Herald added a project: clang.

Clang-tidy can ignore most formatting concerns as formatting is clang-format's job. However, clang-format treats blank lines as intentional separators, so clang-tidy fixes that accidentally insert blank lines can prevent clang-format from properly fixing formatting. For example, applying readability-redundant-member-init to:

  struct F10 {
    F10() :
      f(),
      g()
    {}
    S f, g;
  };

results in:

  struct F10 {
    F10()
  
  
    {}
    S f, g;
  };

which may get converted to:

  struct F10 {
    F10()
  
  
  = default;
    S f, g;
  };

The newly blank lines prevent clang-format from fixing this to the desired:

  struct F10 {
    F10() = default;
    S f, g;
  };

A few individual fixes take steps to reduce some newly blank lines, e.g. RedundantControlFlowCheck.cpp calls `Lexer::findLocationAfterToken(.../*SkipTrailingWhitespaceAndNewLine=*/true )` to chomp trailing newlines after redundant `continue;` or `break;` statements. But this approach is insufficient when multiple rules combine to generate a blank line and forces more work onto each fix writer.

This patch adds a function `removeNewlyBlankLinesFromReplacements` to ClangTidy.cpp that looks for sequences of Replacements that will result in making a previously non-blank line blank. When found a Replacement is added that removes the entire line, including newlines.

The patch adds the above and other tests to readability-redundant-member-init.cpp, and adds tests to readability-redundant-control-flow.cpp and readability-redundant-declaration.cpp to demonstrate its effectiveness in avoiding newly blank lines anywhere.

Two previously private/static helper functions are exposed to assist in the implementation: AST/CommentLexer's `skipNewline` and AST/CommentParser's `isWhitespace(StringRef)`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68682

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-control-flow.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
  clang/include/clang/AST/CommentLexer.h
  clang/include/clang/AST/CommentParser.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68682.223975.patch
Type: text/x-patch
Size: 12258 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191009/154555d3/attachment-0001.bin>