Add clang-tidy checks for Firestore #1078
Conversation
* typedef -> using
* const ref + rvalue ref -> pass by value
* NULL -> nullptr
* remove useless default initializations
* remove useless const value-type parameter declarations (definitions
can still use them)
* use auto instead of repeating types in a cast
| @@ -72,7 +72,7 @@ class Serializer { | |||
| // TODO(rsgowman): If we never support any output except to a vector, it may | |||
| // make sense to have Serializer own the vector and provide an accessor rather | |||
| // than asking the user to create it first. | |||
| static util::Status EncodeFieldValue( | |||
There was a problem hiding this comment.
could you elaborate why remove static here?
There was a problem hiding this comment.
The tests instantiate the serializer and then access these static methods through the instance, which causes a warning under clang-tidy.
In the fullness of time the serializer will need to be stateful (for example, to encode a DocumentKey you need to know the current DatabaseId) so fixing those warnings by removing the static seemed like the better way.
| @@ -47,7 +47,7 @@ class ComparatorHolder { | |||
| template <typename C> | |||
| class ComparatorHolder<C, true> : private C { | |||
| protected: | |||
| explicit ComparatorHolder(const C&) noexcept { | |||
| explicit ComparatorHolder(const C& /* comparator */) noexcept { | |||
There was a problem hiding this comment.
Maybe put the parameter name outside of comment like
explicit ComparatorHolder(const C& comparator) noexcept {
There was a problem hiding this comment.
That causes a different warning about an unused parameter. This way both the compiler and the linter are happy.
I could also do
explicit ComparatorHolder(const C& comparator) noexcept {
(void)comparator;
}But considered this to be inferior. I have no strong preference though.
* Add a .clang-tidy configuration for Firestore C++
* Fix clang-tidy warnings
* typedef -> using
* const ref + rvalue ref -> pass by value
* NULL -> nullptr
* remove useless default initializations
* remove useless const value-type parameter declarations (definitions
can still use them)
* use auto instead of repeating types in a cast
* Fix typos
* Address use of static method through instance warnings
* Address use after move warnings
For now this is only really visible via CLion, when you disable the "use IDE configure" for the clang-tidy inspection.
I've run out of time trying to get CMake to also run these checks though this is something I also want to do.