Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the errors #4

Closed
bombsimon opened this issue Sep 18, 2018 · 8 comments
Closed

Fix the errors #4

bombsimon opened this issue Sep 18, 2018 · 8 comments
Assignees

Comments

@bombsimon
Copy link
Owner

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

@bombsimon bombsimon changed the title Fix the errors? Fix the errors Oct 8, 2019
@bombsimon
Copy link
Owner Author

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 bombsimon self-assigned this Oct 8, 2019
@stmuk
Copy link

stmuk commented Nov 5, 2019

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!

@bombsimon
Copy link
Owner Author

@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
Copy link
Owner Author

bombsimon commented Nov 24, 2019

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:

  • Add a newline before statements requiring a newline
  • 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.

@LennyPenny
Copy link

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

@bombsimon
Copy link
Owner Author

@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
Copy link
Owner Author

bombsimon commented Feb 24, 2020

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 bombsimon mentioned this issue Feb 24, 2020
13 tasks
@bombsimon
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants