cpp-linter / cpp-linter-action

A Github Action for linting C/C++ code integrating clang-tidy and clang-format to collect feedback provided in the form of file-annotations, thread-comments, workflow step-summary, and Pull Request reviews.
https://cpp-linter.github.io/cpp-linter-action/
MIT License
87 stars 20 forks source link

Option to allow acting against different files for clang-tidy and clang-format #233

Closed mirenradia closed 3 weeks ago

mirenradia commented 2 months ago

This is a small feature request to add the capability to act against different files for clang-tidy and clang-format in the same job.

Motivation

In the C++ project I am using this action against, we sometimes separate out the declaration of templates (e.g. a MyTemplateClass.hpp file) and implementations of these templates (e.g. MyTemplateClass.impl.hpp). The .impl.hpp files shouldn't be linted on their own as they only make sense when included directly in the template declaration file and they contain an include guard to that effect such as

#ifndef MYTEMPLATECLASS_HPP_
#error "This file should only be included through MyTemplateClass.hpp"
#endif

#ifndef MYTEMPLATECLASS_IMPL_HPP_
#define MYTEMPLATECLASS_IMPL_HPP_

<implementation code here>

#endif

This leads to cpp-linter reporting errors such as

/path/to/MyTemplateClass.impl.hpp:x:y: error: no template named 'MyTemplateClass' [clang-diagnostic-error]
   x | inline MyTemplateClass<T>::foo(
     |

which I'd like to avoid.

Other options I've considered to get around this:

I'm open to other suggestions if there's already a way to get around the issue I describe that I haven't thought of.

2bndy5 commented 2 months ago

Sounds like you want a tool-specific file filter, yes?

This seems reasonable. And I've been re-thinking about adding glob support in the ignore option. Using globs seems more feasible than using regular expression patterns (specified in yaml as a single-line string).

My first impression is to have ignore-tidy and ignore-format options that accept a value similar to ignore but are only applied according to the tool that is being invoked.

2bndy5 commented 2 months ago

Running separate jobs for clang-format and clang-tidy and doing the above: I think this would work but it seems a little wasteful and I quite like the fact I can do both formatting and linting in 1 job with this action.

Currently, this is the only way to achieve what you want done. I used to do this on a project where certain platforms followed different style guides. However, this is not the best idea for producing digestible output (thread-comments, tidy-review, etc.)

mirenradia commented 2 months ago

Sounds like you want a tool-specific file filter, yes?

This seems reasonable. And I've been re-thinking about adding glob support in the ignore option. Using globs seems more feasible than using regular expression patterns (specified in yaml as a single-line string).

My first impression is to have ignore-tidy and ignore-format options that accept a value similar to ignore but are only applied according to the tool that is being invoked.

I don't want to ignore the files completely (like e.g. a header provided by an external library) as I want to see clang-tidy warnings that appear from them when they are included elsewhere but I don't want clang-tidy to act on them directly if that makes sense?

2bndy5 commented 2 months ago

as I want to see clang-tidy warnings that appear from them when they are included elsewhere but I don't want clang-tidy to act on them directly if that makes sense?

This makes sense to me.

- uses: cpp-linter/cpp-linter-action@v2
  with:
    ignore: .github          # files ignored by both tidy and format
    ignore-tidy: '*.impl.h'  # files ignored by only clang-tidy
    ignore-format: '*.h.in'  # flies ignored by only clang-format