[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Alex Strelnikov via Phabricator via llvm-commits]

astrelni added a comment.

In D62977#1559344 <https://reviews.llvm.org/D62977#1559344>, @lebedev.ri wrote:

> In D62977#1559334 <https://reviews.llvm.org/D62977#1559334>, @astrelni wrote:
>
> > In D62977#1556346 <https://reviews.llvm.org/D62977#1556346>, @lebedev.ri wrote:
> >
> > > In D62977#1540184 <https://reviews.llvm.org/D62977#1540184>, @lebedev.ri wrote:
> > >
> > > > Without seeing the tests - what version checks does this have?
> > > >  It shouldn't fire if the googletest version is the one before that rename.
> > >
> > >
> > > I don't believe this question was answered.
> >
> >
> > Sorry, the original comment got lost between me updating two patches as I noticed I did it wrong without seeing your comment.
> >
> > Unfortunately there are no versioning macros in googletest, so I'm not sure how to keep this check from providing warnings if it is run while depending on a version that is too early. The new names are in master and will be part of an upcoming version 1.9 release. I have tried to update the documentation to clarify which version of googletest this intended for. Please let me know how you think we should proceed.
>
>
> I'm not fully current on what can and can't be done in clang-tidy, but i suppose if the
>  check has matched something, then it must mean that the googletest headers were parsed.
>  Can you perhaps look in the AST that the new names are present?
>  E.g. the new macros, specified in `getNewMacroName()`.


Yes, makes sense. Let me know what you think of the approach I've added in the latest diff. I think it is sufficient to check for the existence of one of the new macros in the right file and one new method in each matched class.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62977/new/

https://reviews.llvm.org/D62977