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

Formally introduce a class of errors that can be automatically fixed #17584

Open
andrewrk opened this issue Oct 18, 2023 · 2 comments
Open

Formally introduce a class of errors that can be automatically fixed #17584

andrewrk opened this issue Oct 18, 2023 · 2 comments
Labels
accepted This proposal is planned. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

We already have such a class of errors. Nearly every version of Zig has released with zig fmt having the capability to automatically convert old syntax to new syntax, for example renaming @fabs to @abs. If zig fmt is not run on such source files, a compilation error would occur.

This proposal is to embrace this concept and take it a step further. Here are some errors that I think belong in this category:

  • Inconsistent usage of tabs and spaces. For example, indentation via spaces on line 1 and indentation via tabs on line 2.
  • Tabs used for whitespace other than indentation
  • Unused locals
  • Pointless discard of locals
  • Mismatched/misleading indentation
  • Unused non-pub globals
  • Source file encoded in a format other than UTF-8
  • Local variable is not mutated
  • Syntax upgrades (e.g. 0.11.0 to 0.12.0)
  • A lint that is missing a corresponding comment to silence it

While a careful programmer might want to know about the occurrence of such problems, all of these do have an obvious solution that can be performed automatically.

Proposed action items:

  1. AstGen should mark such errors as fixable, and continue lowering to ZIR as if no error occurred.
  2. When Semantic Analysis encounters such errors, it should continue with analysis, collecting the rest of the errors and reporting them along with the fixable ones, rather than only the fixable ones being reported and blocking the rest of compilation.
  3. The compiler CLI should expose an --autofix flag that opts into these fixups. When this flag is enabled, errors in this category do not cause compilation to halt. Instead, the source files of the compilation are mutated in place, and compilation proceeds as if the original input had the fixups applied. If any error occurs applying fixups, the compilation fails the same way as if the fixups were not applied.
  4. zig fmt should ignore a subset of errors in this category, such as anything to do with invalid whitespace that will be fixed by zig fmt itself.
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Oct 18, 2023
@andrewrk andrewrk added this to the 0.13.0 milestone Oct 18, 2023
@Srekel
Copy link

Srekel commented Oct 18, 2023

I think autofmt for unused variables actually work pretty well, and am happy to see that the idea is to add autofix fixes for these issues before they become actual compile errors (if I understand you correctly).

Things I just wanted to point out because I hit upon them from time to time:

  1. Loop captures that go from being mutated to not mutated. Currently you need to switch *myvar to myvar. I believe the idea is to auto-detect if something should be const or var, and this just seems like the same thing really ("Local variable is not mutated"), so hoping this gets the autofix treatment.

  2. Similarly it's quite common for me to use a for-loop index capture (for (items) |item, i|), and I need to remove its usage because reasons. Then I also need to remove my index from the for loop. And then I need to add it back. Maybe it could change it to _i? I seem to remember that being up for discussion at some point but wasn't accepted, so I'm guessing this might not be autofix-able.

  3. Labels that are no longer used. This happens from time to time (seldomly) where I temporarily remove a break :LBL. Even though it's rare, it's quite cumbersome because you need to go up to the loop label, and basically make a copy of the line, comment it out, then in the line itself remove the label. Or you can just remove the label, and then type it back in again when you re-add the label reference. In this case it's not clear what the autofmt would do in a clean way, but thought I'd bring it up at least.

I know it's been said before but to me a lot of the frustration from these issues comes from having made a temporary change to code that would still work if it would compile (i.e. it's not a "real" (super scare quotes) compile error), so I compile, only for me to have to look through the compile errors to see what the problem was, sigh, and remove some code that I've written that isn't really causing a problem, compile again, and then run it. Then, I need to do the whole thing backwards again. It not only increases the time it takes to code something to running the game from a second to dozens of seconds, it also makes me lose focus. But having autofix fix the code automatically is good enough! :)

@andrewrk andrewrk added the accepted This proposal is planned. label Oct 18, 2023
@il-k
Copy link

il-k commented Dec 7, 2023

  1. Loop captures that go from being mutated to not mutated. Currently you need to switch *myvar to myvar. I believe the idea is to auto-detect if something should be const or var, and this just seems like the same thing really ("Local variable is not mutated"), so hoping this gets the autofix treatment.

Which one is the mistake in all cases – the capture, or the mutation?

I think it would be better if the error instead pointed to locations of all candidates for decision.
instead of main.zig:4:9: error: cannot assign to constant and an autofix, it would be better if it said something along the lines of

main.zig:4:9: error: cannot assign to constant.
  main.zig:3:15: capture `a` here is constant.
  main.zig:1:10: `as` here is constant.

Now each location is clearly marked and you can jump to whichever you need to make the decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

3 participants