POC: Include GitLab CI code quality feedback in the MR diff view
Problem to solve
GitLab allows code quality tools to display results in the diff view: https://docs.gitlab.com/ee/user/project/merge_requests/code_quality.html#code-quality-in-diff-view
The VS Code extension doesn't surface code quality.
Proposal
Create POC that would expose some code quality metrics in the MR review view in GitLab Workflow
Result of the POC
This document shows the proof of concept of how we could integrate Code Quality reports into VS Code.
In this document I'll look at the available code quality report, available VS Code API for surfacing this report and then I'll suggest possible implementations.
TL;DR
- Check out the videos of possible implementations for normal editor view and MR diff view.
- The POC uses downloaded json artefact, there's no API ATM.
- If we want to enrich the normal editor view (not MR diff), we'll have a hard time showing the errors if the text files have added/removed lines since the report was generated.
- code is in
code-quality-poc
branch
Code Quality Report
I used the gl-code-quality-report.json
JSON artefact from code_quality
job
The report is an array of error messages:
{
"engine_name": "structure",
"fingerprint": "a54cbeb36914a3fde6c04a1f5fb57155",
"categories": [
"Complexity"
],
"check_name": "method_complexity",
"content": {
"body": "# Cognitive Complexity\nCognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.\n\n### A method's cognitive complexity is based on a few simple rules:\n* Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one\n* Code is considered more complex for each \"break in the linear flow of the code\"\n* Code is considered more complex when \"flow breaking structures are nested\"\n\n### Further reading\n* [Cognitive Complexity docs](https://docs.codeclimate.com/v1.0/docs/cognitive-complexity)\n* [Cognitive Complexity: A new way of measuring understandability](https://www.sonarsource.com/docs/CognitiveComplexity.pdf)\n"
},
"description": "Function `parseQuery` has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.",
"location": {
"path": "src/search_input.js",
"lines": {
"begin": 6,
"end": 88
}
},
"other_locations": [],
"remediation_points": 250000,
"severity": "minor",
"type": "issue"
},
Each error message contains description
, severity
, and location
of the problem amongst a few extra details.
VS Code API
Diagnostic](https://code.visualstudio.com/api/references/vscode-api#Diagnostic) interface allows us to put error/warning/info messages on any location in any file (including MR diff), this is the Errors & Warnings feature from the user perspective.
Diagnostic consists of location in the code, severity, message and source. All visible on the following screenshot:
Possible implementation
MR diff view
We would show the code quality information in the MR review Diff. Showing every error from the gl-code-quality-report.json
doesn't make sense here because the MR author is not responsible for most of the issues. Here we would need an API endpoint that exposes the diff (existing/new errors) for each MR. (We could also download the artefact from main
branch and from the MR branch and diff them, but that sounds like a lot of effort.)
Normal editor view
We can also just add all the Code Quality errors directly into user's editor which will highlight problems in the file tree, show them in the editor and also in the "Problems" output panel at the bottom. This works well even with the whole gl-code-quality-report.json
(except for causing too much noise).
- We could have a right-click context menu (e.g. "show code quality") on each pipeline with
code-quality
job. This right-click would download the artefact and show all the errors. - We could only download the errors that are newly introduced by the MR (when there's an API endpoint) and show them.
General problems
Too much noise
Highlighting whole blocks of code is very intrusive compared to normal warnings (e.g. eslint)
very targetted warning by most of the extensions | code quality marking the whole file as having too many lines |
---|---|
On top of highlighting the whole blocks, the gl-code-quality-report.json
only contains lines on which the problem occurs, not columns. So if an error applies only to a symbol or two, we still highlight the whole line.
{
"location": {
"path": "src/search_input.js",
"lines": {
"begin": 6,
"end": 88
}
},
}
Possible solutions to the "Too Much Noise" problem
- show the error/warning on the first line of the complexity issue, contributed by @KevSlashNull (#410 (comment 638971740))
- not showing complexity errors (and other kinds that would highlight 10+ lines) or at least not by default
- add an easy way to toggle the visibility of these errors (adding a button to the editor or to the status bar)
Making sure the report is are up to date
VS Code handles moving the Diagnostic
highlights with changes, but it incorrectly highlights if the changes happened before we generated the Diagnostic
Diagnostic objects generated before changes | Diagnostic object generated after changes |
---|---|
errors-before-changes | errors-after-changes |
VS Code can automatically track the error even when there are new/removed lines | VS Code highlights wrong lines, because it can't know that we are giving diagnostic for different version of the file |
Mapping Code Quality report error on diagnostic
There are a few attributes in the code quality report, which don't have a natural place in the Diagnotic
object.
engine_name
check_name
-
categories
would have to be somehow included in the message or the source.
eslint
includes this in the source part ( @typescript-eslint/no-unused-vars
)
Severity
It seems that we've got only two severities in the report minor
/ major
but VS Code offers four:
severity | visual |
---|---|
error | |
warning | |
info | |
tip |