[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:24
+
+  const auto IsDuration =
+      expr(hasType(cxxRecordDecl(hasName("::absl::Duration"))));
----------------
maybe call it `DurationExpr` since you have declared the variable as `expr(...)`.


================
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:30
+              hasOverloadedOperatorName("/"),
+              hasArgument(0, expr(IsDuration)),
+              hasArgument(1, expr(IsDuration))).bind("OpCall"))),
----------------
`expr(IsDuration)` seems have a duplicated `expr`, isn't `hasArgument(0, IsDuration())` enough?


================
Comment at: test/clang-tidy/abseil-duration-division.cpp:18
+
+absl::Duration d;
+
----------------
could you make it to a local variable? It willmake the test code easier to read, otherwise readers have to scroll up the file to see what is variable `d`.


================
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+  // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+  // absl::FDivDuration(t1, t2);}
----------------
I think we should ignore templates. The template function could be used by other types, fixing the template is not correct. 


https://reviews.llvm.org/D50389