[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:376
+    // expression wouldn't really benefit readability. Therefore we abort.
+    if (NewReturnLength > MaximumLineLength) {
+      return;
----------------
Is there really no way to format the fixes, and *then* check the line length?
```
$ clang-tidy --help
...
  -format-style=<string>       - 
                                 Style for formatting code around applied fixes:
                                   - 'none' (default) turns off formatting
                                   - 'file' (literally 'file', not a placeholder)
                                     uses .clang-format file in the closest parent
                                     directory
                                   - '{ <json> }' specifies options inline, e.g.
                                     -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
                                   - 'llvm', 'google', 'webkit', 'mozilla'
                                 See clang-format documentation for the up-to-date
                                 information about formatting styles and options.
                                 This option overrides the 'FormatStyle` option in
                                 .clang-tidy file, if any.
...
```
so `clang-tidy` is at least aware of `clang-format`.


================
Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.h:30
+      : ClangTidyCheck(Name, Context),
+        MaximumLineLength(Options.get("MaximumLineLength", 100)) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
It would be better to default to `80`, especially if it can't be read from `.clang-tidy`.


================
Comment at: test/clang-tidy/readability-unnecessary-intermediate-var.cpp:172
+bool f_long_expression() {
+  auto test = "this is a very very very very very very very very very long expression to test max line length detection";
+  return (test == "");
----------------
Please add edge-cases, i.e. just below `MaximumLineLength`, exactly `MaximumLineLength`, ...


https://reviews.llvm.org/D37014