Rahn
Rahn

Reputation: 5405

clang-tidy check: write a matcher to match consective if-else-if statement?

I'm modifying clang-tidy to build a matcher to match multiple (more than 3) else if statements, like such:

    if (val < 0) {
        result = 0;
    }
    else if (val < 1) {
        result = 1;
    }
    else if (val < 2) {
        result = 2;
    }
    else if (val < 3) {
        result = 3;
    }
    else {
        result = 4;
    }

To begin with I try to match the simplest case with 2 nested if:

    if (condition_a) {
    }
    else if (condition_b) {
    }

So I have my registerMatchers() and check():

void MultiIfElseCheck::registerMatchers(MatchFinder *Finder) {
  Finder->addMatcher(ifStmt(hasThen(hasDescendant(ifStmt()))).bind("outer_if"),
                     this);
}

void MultiIfElseCheck::check(const MatchFinder::MatchResult &Result) {
  const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>("outer_if");

  if (OuterIf) {
    diag(OuterIf->getIfLoc(), "This IF is followed with too many ELSE-IF\n",
         DiagnosticIDs::Note);
  }
}

But this matcher matched nothing from the code above. I try futher simplify the matcher to any ifStmt:

Finder->addMatcher(ifStmt().bind("outer_if"), this);

And still matched nothing.

Can someone tell me how do I fix it?

Upvotes: 0

Views: 161

Answers (1)

Scott McPeak
Scott McPeak

Reputation: 12749

To match an if-else chain, use a matcher like:

ifStmt(
  hasElse(
    ifStmt(
      hasElse(
        ifStmt(
          hasElse(
            ifStmt(
            )
          )
        )
      )
    )
  )
).bind("outer_if"),

The main issue with your attempt is using hasThen rather than hasElse. With hasThen, it would match things like:

  if (...) {
    if (...) {}      // 'if' inside the "then" clause
  }

I can't explan why the trivial ifStmt() matcher doesn't match anything. It does for me.

Here's a complete example clang-tidy checker that reports the example in the question:

// HelloCheck.cpp

#include "HelloCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang-tidy/ClangTidyModule.h"          // ClangTidyModule
#include "clang-tidy/ClangTidyModuleRegistry.h"  // ClangTidyModuleRegistry

using namespace clang::ast_matchers;

namespace clang {
namespace tidy {

void HelloCheck::registerMatchers(MatchFinder *Finder) {
  Finder->addMatcher(
    ifStmt(
      hasElse(
        ifStmt(
          hasElse(
            ifStmt(
              hasElse(
                ifStmt(
                )
              )
            )
          )
        )
      )
    ).bind("outer_if"),
    this
  );
}

void HelloCheck::check(const MatchFinder::MatchResult &Result) {
  const auto *node = Result.Nodes.getNodeAs<IfStmt>("outer_if");
  diag(node->getBeginLoc(),
       "if stmt");
}

class HelloModule : public ClangTidyModule {
public:
  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
    CheckFactories.registerCheck<HelloCheck>(
        "hello-check");
  }
};

static ClangTidyModuleRegistry::Add<HelloModule> X("hello-module",
                                                   "Adds HELLO THERE checks.");

int clangTidyMain(int argc, const char **argv);

} // namespace tidy
} // namespace clang

int main(int argc, const char **argv)
{
  return clang::tidy::clangTidyMain(argc, argv);
}

(My main function is perhaps a bit dodgy, but that's unrelated to this question.)

When given the input:

// test.cc
// Exercise cases of nested 'if' statements.

int result;

void f(int val)
{
    if (val < 0) {
        result = 0;
    }
    else if (val < 1) {
        result = 1;
    }
    else if (val < 2) {
        result = 2;
    }
    else if (val < 3) {
        result = 3;
    }
    else {
        result = 4;
    }
}

// EOF

Its output is:

$ ./HelloCheck.exe '-checks=-*,hello-check' test.cc --
1 warning generated.
$(PWD)/test.cc:8:5: warning: if stmt [hello-check]
    if (val < 0) {
    ^

Note: If there are more than three else if elements in the chain, then you will get matches for all that have three or more after them. More work would be needed to filter out all but the first one.

Upvotes: 0

Related Questions