chore: make guided remediation follow revive's default lint rules

[michaelkedar]

Helping with #1257
Changed the guided remediation code (cmd/osv-scanner/fix, and internal/resolution, remediation and tui) to fix lint errors found when using revive's default settings. All of this is internal, so there's no API breakages.

It was mostly unexported-return and stuttering complaints (e.g. resolution.ResolutionResult -> resolution.Result), so a bunch of structs have been renamed.

[michaelkedar]

I've had the tendency to name these variables rw (for read-write).
It might make sense to rename the structs manifest.ReadWriter and lockfile.ReadWriter.
Any thoughts/opinions?

[cuixq]

I don't have preference between the two options, both SGTM.

[michaelkedar]

I've gone and renamed them to ReadWriters

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 64.18605% with 77 lines in your changes missing coverage. Please review.

Project coverage is 68.30%. Comparing base (1856add) to head (91f842d).

Files with missing lines	Patch %	Lines
internal/tui/vuln-list.go	0.00%	12 Missing ⚠️
scripts/generate_mock_resolution_universe/main.go	0.00%	10 Missing ⚠️
cmd/osv-scanner/fix/state-relock-result.go	0.00%	8 Missing ⚠️
cmd/osv-scanner/fix/state-choose-strategy.go	0.00%	7 Missing ⚠️
internal/tui/vuln-info.go	0.00%	7 Missing ⚠️
cmd/osv-scanner/fix/state-in-place-result.go	0.00%	6 Missing ⚠️
cmd/osv-scanner/fix/noninteractive.go	82.75%	2 Missing and 3 partials ⚠️
cmd/osv-scanner/fix/model.go	0.00%	4 Missing ⚠️
internal/resolution/datasource/npm_registry.go	62.50%	3 Missing ⚠️
internal/tui/relock-info.go	0.00%	3 Missing ⚠️
... and 8 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1259      +/-   ##
==========================================
+ Coverage   68.29%   68.30%   +0.01%     
==========================================
  Files         175      175              
  Lines       16764    16764              
==========================================
+ Hits        11449    11451       +2     
+ Misses       4682     4681       -1     
+ Partials      633      632       -1     

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

[G-Rath]

fwiw I've written up my thoughts on this on the issue - overall my main comment here is that I think it would be best to see if you can get the now-not-violated revive rules enabled for these files/packages to prevent future regressions and to let us know what in these files still needs actioning via explicitly inline disables.

[cuixq]

Thanks Michael!

[cuixq]

I feel we can have a better name.. but I don't have a good suggestion neither...

[michaelkedar]

Yeah, it's not great. I settled on OSV because it is the format of the structure.

[michaelkedar]

overall my main comment here is that I think it would be best to see if you can get the now-not-violated revive rules enabled for these files/packages to prevent future regressions and to let us know what in these files still needs actioning via explicitly inline disables.

I can't seem to work out good a way to only enable revive rules for specific files (apparently you can add an exclude: do individual rules, but I can't get that to work.

Best I can do is add something like

issues:
  exclude-rules:
    - path-except: internal/(resolution|remediation|tui)/
      linters:
        - revive

But that's not great, and it'll disable the indent-error-flow lint that we currently have enabled for everything else...

I can go disable the violating rules inline everywhere else, but tbh it'd be easier to fix most of the violations instead (and I wanted to limit the scope of this PR)