feat: introduce problem severity levels

[maxim-tschumak]

Issues with API definitions might be of different criticality. Some issues are of informative character and some are very critical.

To differentiate we can introduce problem severity field. It can be used to, for example, decide whether a violation should be blocking (not allowed) or not (informative).

[noahdietz]

Ok so I noticed that we have a Category string field and it was added for a similar reason (see #82). What I don't understand is why there aren't any pre-defined categories. There is a reasoning stated in that issue is as follows:

it is up to users to define them in their configuration, so they can develop/use downstream tools to target their categories

So I am wondering if we should do one of the following:

1. Go with this PR to add Severity as a purpose-built field
2. Introduce some pre-defined Category values that emulate Severity publicly
3. Introduce some pre-defined Category values that emulate Severity internally (which our analyzers would key off of when reporting to critique)

WDYT? I can see the benefit of adding a Severity "enum", but I'm also worried about bloat considering it is slightly related to Category.

@GregFurmanek WDYT?

[maxim-tschumak]

Consider also the following: It may make more sense to make the severity part of the linter configuration. By doing so, we would enable teams to decide how they want API owners to react to the problems in their (e.g. project) scope. Basically a config entry becomes (files, rules, severity) tuple and can be defined multiple times.

[noahdietz]

we would enable teams to decide how they want API owners to react to the problems in their (e.g. project) scope.

I don't think we want to give individual API teams (not PAs, but individual APIs within a PA) the ability to downgrade a linter warning. If they are exempt they can disable the rule in the proto with a comment or in their config. But the rule author should decide how serious a linter warning is IMO. Also, I'd be worried about config bloat/complexity. These configs should be dead simple to look at and interpret.

We should keep a severity style attribute on the lint.Problem.

[GregFurmanek]

I agree with Noah. Linter is an automated test tool of sorts. Tests are pretty dimple in their function and answer one question: does the code pass specific scenario. The concept of severity is more useful in error reporting. Not so much in testing.

[noahdietz]

I agree with Noah. Linter is an automated test tool of sorts. Tests are pretty dimple in their function and answer one question: does the code pass specific scenario. The concept of severity is more useful in error reporting. Not so much in testing.

I might not have been clear above, but, I don't disagree with adding a severity-style field to lint.Problem that we can use in Tricorder to indicate if the finding is Blocking or not. I just believe it should be a property of lint.Problem, and not in the lint.Config, because the author of the rule should decide if the finding is blocking or not.

[maxim-tschumak]

[..] should be a property of lint.Problem, because the author of the rule should decide if the finding is blocking or not

Another way of seeing it is that the severity is a governance aspect which is usually ruled by organization. In that case, linter operator (i.e. a (sub-)org) should decide.