[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 a comment.

Thank you for working on this!
Some thoughts below.



================
Comment at: clang-tidy/readability/CMakeLists.txt:31
   UniqueptrDeleteReleaseCheck.cpp
+  UnnecessaryIntermediateVarCheck.cpp
 
----------------
Please upload patches with full context (`-U999999`)


================
Comment at: clang-tidy/utils/LexerUtils.h:26
+/// Get source code text for statement.
+Optional<StringRef> getStmtText(const Stmt* Statement, const SourceManager& SM);
+
----------------
This should probably be split into new differential, and it should have a unit-test.
(And mark that diff as parent of this one)


================
Comment at: clang-tidy/utils/Matchers.h:54
+// Matches the next statement within the parent statement sequence.
+AST_MATCHER_P(Stmt, hasSuccessor,
+              ast_matchers::internal::Matcher<Stmt>, InnerMatcher) {
----------------
This should be split into another new differential, and it should have a unit-test.
(And mark that diff as parent of this one)
Also, the docs will need to be updated, which is sadly impossible now, see D41455.


================
Comment at: test/clang-tidy/readability-unnecessary-intermediate-var.cpp:1
+// RUN: %check_clang_tidy %s readability-unnecessary-intermediate-var %t
+
----------------
Still no tests for macros, preprocessor definitions.
I do see that it will still do the wrong thing, but i need to see that in the tests and release notes.


https://reviews.llvm.org/D37014