wsl icon indicating copy to clipboard operation
wsl copied to clipboard

Fix the errors

Open bombsimon opened this issue 5 years ago • 7 comments

Support to create a fixed version like go fmt to resolve the reported issues.

bombsimon avatar Sep 18 '18 16:09 bombsimon

I'll try to fix this this week but I have to dig into how golangci-lint want this data so I can implement it there at the same time.

bombsimon avatar Oct 08 '19 20:10 bombsimon

This would be very useful! I generally like the whitespace suggestions but it's a lot of effort to fix them manually and an automatic "fixup" mode would be great!

stmuk avatar Nov 05 '19 16:11 stmuk

@stmuk Yeah I totally agree! I've just scratched the surface on how golangci-lint handles this but it seems like if I implement it the way golangci-lint want it it's not obvious if I can support it natively in wsl.

I'll try to find the time to look into this and I think it would be a lot more useful for people out there if the tool was bundled with a way to apply the suggestions.

bombsimon avatar Nov 05 '19 18:11 bombsimon

I finally got the time to have a look at this and the obvious way forward seems to be to use analysis with TextEdit.

I've created a the branch v2.1.0-fixer where the work will be done. Any help would be appreciated!

I must say I feel a bit lost looking into this but I've made some progress. The major part of adding a newline before a cuddled command is done but I must figure out a way to format the file properly. The most important part would be to remove the tabs used as indentation and add them back again. Not really sure how I would do this but I guess I'll take what's between the previous statement + potential comments and the faulty statement.

So the far:

  • [x] Add a newline before statements requiring a newline
  • [x] Make the newlines correctly indented (take comments and other things in consideration)
  • [ ] Figure out why no all errors are handled, caching, large projects etc.*
  • [ ] Advanced formatting? (E.g. multiple var group instead of with spaces between)
  • [ ] Remove newlines in beginning or end of blocks

I'm trying to run this code on bigger projects and it seems like not all errors are found and also that the numbers of errors varies.

for _ in {1..10}; do wsl-analyze ./... 2>&1 | wc -l; done
     350
     325
      19
     276
      17
     348
      77
     328
     134
     234

After running with the -fix flag several times it seems to be a consistent 0 but wsl still finds and reports errors which never get's detected by the analyser.

bombsimon avatar Nov 24 '19 20:11 bombsimon

Could you convert this to a tracking pull request?:)

LennyPenny avatar Feb 24 '20 10:02 LennyPenny

@LennyPenny Sorry but I'm not familiar with what that means? Should I just create a working PR towards master or does it mean something else?

bombsimon avatar Feb 24 '20 11:02 bombsimon

It seems like my naive approach of passing all analasys.Pass Files to one function was stupid so instead I now create a new processor for each file (which is a subset of the AST) and that seems to be consistent. Output from the same project as above.

$ for _ in {1..10}; do wsl-analyzer ./... 2>&1 | wc -l; done
    1087
    1087
    1087
    1087
    1087
    1087
    1087
    1087
    1087
    1087

So now the next step would be to figure out a proper way to handle comments. Since the analasys package already parsed the AST there's not much I can do, however I recently found dave/dst which handles comments much much better.

So given that I can convert the AST to DST I guess this might actually not be so difficult.

So sorry for this issue not being resolved but I hope I can get the time and motivation to actually finish this fixer which I guess would help people a lot.

PS. Any input and help is very much welcome!

bombsimon avatar Feb 24 '20 12:02 bombsimon

It almost took 5 years but I guess this just shows that we should never stop hoping. Sorry for the delay and the solution is currently not compatible with golangci-lint (see README or PR).

I'll draft a release soon that can be used to install v4 of wsl with go get so you can run wsl --fix. Until then you can use the latest commit on master or build manually.

bombsimon avatar Mar 15 '23 21:03 bombsimon