clang-format-18 false negative when using function-try block

[Alexolut]

I have code like this, formatted locally with clang-format. I use function-try-block here:

void TestFunc(const std::string& data) try {
    do_something();
    do_something_more();
} catch (const std::exception& ex) {
    log_error(ex);
}

cpp-linter has complaints on this code, pointing to the line with catch (99):

Notice: File filename.cpp does not conform to Custom style guidelines. (lines 99)

At the same time I don't see any diff suggestions in PR despite I have format-review: true and if I intentionally add wrongly-formatted changes on other lines - I will see only diff for those lines (but line with catch still won't be in diff for some reason).

Full list of options that I use:

        uses: cpp-linter/cpp-linter-action@v2.12.0
        id: linter
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        with:
          style: file
          tidy-checks: '-*'
          version: '18'
          step-summary: true
          lines-changed-only: true
          format-review: true
          passive-reviews: true
          thread-comments: update

Looks like everything is OK with formatting, but cpp-linter mark that code as not-formatted.

[2bndy5]

What does the review summary comment say? The format-review feature should attach a full diff of desired changes in the review's summary comment. It would be collapsed under the text "Click here for the full clang-format patch". For example: https://cpp-linter.github.io/cpp-linter-action/images/format-review.png

I have to ask, what triggers are used by your CI workflow? I think I asked you this before but got no answer. For example the triggers are under the yaml field on:

on:
  pull_request:
    branches: [main, master, develop]
    paths: ['**.c', '**.cpp', '**.h', '**.hpp', '**.cxx', '**.hxx', '**.cc', '**.hh', '**CMakeLists.txt', 'meson.build', '**.cmake']
  push:
    branches: [main, master, develop]
    paths: ['**.c', '**.cpp', '**.h', '**.hpp', '**.cxx', '**.hxx', '**.cc', '**.hh', '**CMakeLists.txt', 'meson.build', '**.cmake']

Also, which lines are changed in the following code?

void TestFunc(const std::string& data) try {
    do_something();
    do_something_more();
} catch (const std::exception& ex) {
    log_error(ex);
}

[Alexolut]

In general I don't have problems with other code. That's why I think the triggers not relevant here.

Only the line with catch is problematic from cpp-linter point of view. All lines of code that I mentioned are new. So the entire function is checked.

[2bndy5]

If you don't answer all my questions, then I'm unable to help/diagnose the problem. I'm guessing this has occurred on a private repo.

[2bndy5]

formatted locally with clang-format

Are you also using clang-format v18 locally?

what triggers are used by your CI workflow?

This is important because the review may not be updated based on what event triggers the workflow.

What does the review summary comment say?

I'm curious if the "full clang-format patch" includes the line about catch.

[Alexolut]

The repo is private. Excerpt from related yaml file:

on:
  workflow_call:

I formatted entire file locally with clang-format 18.1.7 (on CI I see 18.1.8 but I had faced similar problem before while CI had 18.1.7 as well) before push to PR.

When I changed code into this (added extra spaces inside of parentheses):

void TestFunc(const std::string& data) try {
    do_something();
    do_something_more(      );
} catch (const std::exception& ex) {
    log_error(   ex    );
}

I got a message with diff in PR review:

I.e. no problems on line with catch are detected in CI side! That's why I think triggers and difference in minor versions of LLVM are not relevant in my case.

When I apply the diff (i.e. format file again locally) I will see:

• no diff on review page
• complaints about line with catch

The only way I found to work around the problem (get no complains from linter side) - don't use function-try-block and use regular try-block like this:

void TestFunc(const std::string& data) {
    try {
        do_something();
        do_something_more();
    } catch (const std::exception& ex) {
        log_error(ex);
    }
}

[2bndy5]

on:
  workflow_call:

So it is a reusable workflow. What are the triggers for the actual workflow that invokes the reusable workflow? I ask because reusable workflows inherit the event type from the calling workflow.

I formatted entire file locally with clang-format 18.1.7 (on CI I see 18.1.8 but I had faced similar problem before while CI had 18.1.7 as well) before push to PR.

I was more concerned about a difference between major versions. I doubt this problem is occurring from a difference between patch versions

When I apply the diff (i.e. format file again locally) I will see:

• no diff on review page
• complaints about line with catch

It sounds like format-review is working as expected, but you keep seeing a message from the file-annotations feature. Since this isn't the first time you have had complaints about the file-annotations feature (and you're more focused on diff suggestions), I would recommend you disable it:

file-annotations: false

Note

We have no way of deleting an outdated file annotation because they are only relevant to the commit that created the annotation. This usually isn't a problem because refreshing the list of file changes in a PR will only show annotations created by the latest commit to the head branch.

It could be that the line number in the annotation is incorrect, but this is highly unlikely.

[Alexolut]

I disabled annotation as you suggested file-annotations: false. Below the output from action run:

Performing checkup on filename.cpp
  INFO:CPP Linter:Running "/usr/bin/clang-format-18 -style=file --output-replacements-xml --lines=96:103 filename.cpp"
  INFO:CPP Linter:Getting fixes with "/usr/bin/clang-format-18 -style=file --lines=96:103 filename.cpp"
  
Posting comment(s)
  INFO:CPP Linter:1 clang-format-checks-failed
  INFO:CPP Linter:0 clang-tidy-checks-failed
  INFO:CPP Linter:1 checks-failed
  ERROR:CPP Linter:response returned 403 from PUT https://api.github.com/repos/*/reviews/*/dismissals with message: {"message":"Branch protections do not permit dismissing this review","documentation_url":"https://docs.github.com/rest/pulls/reviews#dismiss-a-review-for-a-pull-request","status":"403"}
  ERROR:CPP Linter:response returned 403 from PUT https://api.github.com/repos/*/reviews/*/dismissals with message: {"message":"Branch protections do not permit dismissing this review","documentation_url":"https://docs.github.com/rest/pulls/reviews#dismiss-a-review-for-a-pull-request","status":"403"}

As far as I understand the line INFO:CPP Linter:1 clang-format-checks-failed means that the problem persist. Do have an ability (public sandbox?) to run your cpp-linter with my code snippet?

[2bndy5]

Do have an ability (public sandbox?) to run your cpp-linter with my code snippet?

No. We would need more than just the snippet:

• the .clang-format config file you are using
• any compilation database generated would have to be significantly altered (clang-tidy only)

You can enable cpp-linter's debug output:

verbosity: debug # default is 'info'

If I recall correctly, this should show the output from

/usr/bin/clang-format-18 -style=file --lines=96:103 filename.cpp

[Alexolut]

Look like the output is empty line? But same output here was before changing verbosity to debug.

the .clang-format config file you are using

I hope that default based on Google-style will be enough to reproduce

[2bndy5]

I forgot, clang-format output is basically the modified source. With format-review enabled, the modified source is captured into a buffer that is used to generate a diff (which corresponds to the code suggestions).

Could this just be a difference in LF vs CRLF? git typically normalizes line endings according to .gitattributes and git config settings

[Alexolut]

Could this just be a difference in LF vs CRLF?

I don't think so. All lines have the same ending. But complaining only for the one with catch.

I would like to repeat that the problem not raising with regular try/catch block, only with function-try-block. Looks like clang-format (or other wrapper) returns non-zero exit code to cpp-linter, but doesn't suggest any changes.

[2bndy5]

If clang-format was returning a non-zero exit code, then it would have been reported with debug output enabled.

    logger.info('Running "%s"', " ".join(cmds))
    results = subprocess.run(cmds, capture_output=True)
    if results.returncode:
        logger.debug(
            "%s raised the following error(s):\n%s", cmds[0], results.stderr.decode()
        )

(from src here)

[2bndy5]

What is the output when you run the following command locally?

clang-format --output-replacements-xml -style=file --lines=96:103 filename.cpp

(assuming clang-format-18 is synonymous with a clang-format call)

I'm curious to see what the XML output is. It might be that v18 uses a format that is different from previous versions' XML output (we've seen that problem before).

[2bndy5]

I hope that default based on Google-style will be enough to reproduce

Its not. You clearly use 4-space indents, Google guidelines mandate 2-space indents. I wouldn't be surprised if you deviate from their guidelines further.

[Alexolut]

output-replacement-xml returns me the following:

<?xml version='1.0'?>
<replacements xml:space='preserve' incomplete_format='false'>
<replacement offset='2789' length='0'>&#13;&#10;    </replacement>
</replacements>

Offset 2789 points right after the curly brace:

} catch (const std::exception& ex) {
                                    ^

I'm not sure what does it mean. How to read this xml-output.

You clearly use 4-space indents

I mean that for the sake of reproducibility it (I hope) should not depend on .clang-format style. Just try to format my code snippet with your settings and look into response from cpp-linter, if it's not so hard for you.