Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

request a new option enforce-enum-types #65

Open
fatsheep9146 opened this issue Jul 18, 2023 · 5 comments
Open

request a new option enforce-enum-types #65

fatsheep9146 opened this issue Jul 18, 2023 · 5 comments

Comments

@fatsheep9146
Copy link

In project opentelemetry-collector-contrib, we want to use exhaustive linter to confirm the way of using go.opentelemetry.io/collector/pdata/pmetric.MetricType.

related issue:
open-telemetry/opentelemetry-collector-contrib#7532
open-telemetry/opentelemetry-collector-contrib#23242

However in this project, there will be a lot of code provided by third-party vendors. These third-party code may come with their own enum types that may violate the exhaustive lint requirements. But in reality, these third-party enum types are not something we are concerned about.

What we want to achieve is to only check the go.opentelemetry.io/collector/pdata/pmetric.MetricType enum type while ignoring all other enum types. But for now, there is no option we can achieve, so we have to use --explicit-exhaustive-switch option, manually pick out all places in code where pmetric enum type is used, then add //exhaustive:enforce comment. This method is very prone to omissions and errors since it is manual.

So I wonder if we can support another option like enforce-enum-types, which is just like ignore-enum-types, but the behavior is opposite. And maybe this option should come up with explicit-exhaustive-switch. When explicit-exhaustive-switch is enabled and an enum type A is specified in enforce-enum-types, then all switch or map using enum type A and the switch with //exhaustive:enforce will be checked.

@fatsheep9146
Copy link
Author

@nishanths hi, what do you think about the feature? If it is ok, I'd like to help with this feature.

@nishanths
Copy link
Owner

nishanths commented Aug 1, 2023

I'm +1 for this. Does the following description match what you are thinking?

The flag enforce-enum-types, if set, specifies the enum types for which exhaustiveness should be checked. By default the program checks all enum types. The flag serves to limit the types checked to only the specified types.

Please feel free to send a pull request!

@nishanths
Copy link
Owner

nishanths commented Aug 1, 2023

Could we discuss the following portion from the issue description more?

And maybe this option should come up with explicit-exhaustive-switch. When explicit-exhaustive-switch is enabled and an enum type A is specified in enforce-enum-types, then all switch or map using enum type A and the switch with //exhaustive:enforce will be checked.

Were you suggesting that we might need special (i.e. non-straightforward) handling for the intersection of enforce-enum-types and explicit-exhaustive-switch? If not, feel free to ignore the rest of this comment. If so, as far as I can tell, I think no special handling will be necessary. We have these possible combinations:

enforce-enum-types  explicit-exhaustive-switch
off                 off
off                 on
on                  off
on                  on

The first two combinations (off, off) and (off, on) should already be handled correctly by the present code, since the new enforce-enum-types flag is off. For the third combination (on, off), we would check all switch statements that switch on any of the specified enforce-enum-types types. For the fourth combination (on, on), we would check switch statements marked with //exhaustive:enforce that switch on any of the specified enforce-enum-types types.

@nishanths nishanths changed the title request a new option explicit-exhaustive-switch request a new option enforce-enum-types Aug 1, 2023
@fatsheep9146
Copy link
Author

statements

Thanks for your reply!

Yes, I think the behaviors of four combinations you list are just as same as I understand. I will start to work on this and send a pr ASAP.

@ImprintNav
Copy link

Consider also the approach used in https://github.com/GaijinEntertainment/go-exhaustruct via regexes to allow covering entire packages or subdirectories. There's a lot of analogies between this analyzer and that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants