feat: registery based diagnostician

[ghaith]

A Diagnostic registry is now responsible for adding a severity instead
of the diagnostic itself. The registry is the default diagnostics
assessor. Diagnostics no longer have an own severity field as a result.

The registry can be configured using the --error-config flag where error severity can be adjusted
The current project registry can be printed out using plc config diagnostics

Diagnostics now also show the error number in the message as well as a hint on how to get more information about the error.
The explain subcommand can be used to read information about each error code.

[codecov]

Codecov Report

Attention: Patch coverage is 72.33503% with 109 lines in your changes are missing coverage. Please review.

Project coverage is 95.64%. Comparing base (b3e27f2) to head (364fcbd).

Files	Patch %	Lines
...iagnostics/src/diagnostics/diagnostics_registry.rs	70.78%	26 Missing ⚠️
compiler/plc_diagnostics/src/diagnostician.rs	18.18%	18 Missing ⚠️
compiler/plc_driver/src/cli.rs	75.00%	17 Missing ⚠️
compiler/plc_driver/src/lib.rs	73.91%	12 Missing ⚠️
src/parser/expressions_parser.rs	28.57%	5 Missing ⚠️
src/validation/statement.rs	90.69%	4 Missing ⚠️
compiler/plc_diagnostics/src/diagnostics.rs	91.42%	3 Missing ⚠️
compiler/plc_project/src/project.rs	0.00%	3 Missing ⚠️
compiler/plc_diagnostics/src/reporter/clang.rs	50.00%	2 Missing ⚠️
compiler/plc_driver/src/pipelines.rs	33.33%	2 Missing ⚠️
... and 13 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1077      +/-   ##
==========================================
- Coverage   95.74%   95.64%   -0.11%     
==========================================
  Files         147      148       +1     
  Lines       43286    43455     +169     
==========================================
+ Hits        41446    41561     +115     
- Misses       1840     1894      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[riederm]

overall I like it,
what I dont like is that creating a diagnostic and especially assigning the error code looks like it is very easy to pick a wrong string or a string that does not exist. It also looks like harder to see if we no longer use a certain error. It is also not really streight forward where to look for the details behind E017? btw. is E17 == E017? i guess not since this is a string?
Does the E stand for error?

 Diagnostic::new("A class cannot have an implementation")
                .with_error_code("E017").   // <-- this string
                .with_location(implementation.location.to_owned())

[riederm]

Suggested change

	Error codes are usually provided in the diagnostic report
	Error codes are usually provided in the diagnostic report.

[ghaith]

overall I like it, what I dont like is that creating a diagnostic and especially assigning the error code looks like it is very easy to pick a wrong string or a string that does not exist. It also looks like harder to see if we no longer use a certain error. It is also not really streight forward where to look for the details behind E017? btw. is E17 == E017? i guess not since this is a string? Does the E stand for error?

 Diagnostic::new("A class cannot have an implementation")
                .with_error_code("E017").   // <-- this string
                .with_location(implementation.location.to_owned())

Yes, strings are a problem, I wanted to still find a solution for it, but I felt it is still better than the enum as it allows multiple locations to define errors.
In practice, it's doing the same job so I won't be mad if you insist on enums here :)

[riederm]

overall I like it, what I dont like is that creating a diagnostic and especially assigning the error code looks like it is very easy to pick a wrong string or a string that does not exist. It also looks like harder to see if we no longer use a certain error. It is also not really streight forward where to look for the details behind E017? btw. is E17 == E017? i guess not since this is a string? Does the E stand for error?

 Diagnostic::new("A class cannot have an implementation")
                .with_error_code("E017").   // <-- this string
                .with_location(implementation.location.to_owned())

Yes, strings are a problem, I wanted to still find a solution for it, but I felt it is still better than the enum as it allows multiple locations to define errors. In practice, it's doing the same job so I won't be mad if you insist on enums here :)

Im not insisting, but I'd like to have a discussion. Next time anybody creates a new diagnostics, what is the best way to find out, if there is already an error-number for it? auto-importing all of them in the book would be an option :-/

[riederm]

I would really like to know if we're doing something wrong. The merges appear as new changes in the review and its really confusing that suddenly I see a ton of unrelated changes - or I need to look carefully if they are new and unrelated (by a merge).

[ghaith]

I would really like to know if we're doing something wrong. The merges appear as new changes in the review and its really confusing that suddenly I see a ton of unrelated changes - or I need to look carefully if they are new and unrelated (by a merge).

Not sure, but I think if you chose to see changes since your last review the merges would appear as if they came from this commit. If you see the entire history they will blend in

[riederm]

Not sure, but I think if you chose to see changes since your last review ...

is this wrong? what else would I do? I just wonder if there is a smarter whay of doing this? big project should have this problem every day? maybe they wait for the merge until it is reviewed? idk