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

idea: consult git diff for better diagnostics / syntax errors #48911

Closed
matthiaskrgr opened this issue Mar 10, 2018 · 7 comments
Closed

idea: consult git diff for better diagnostics / syntax errors #48911

matthiaskrgr opened this issue Mar 10, 2018 · 7 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@matthiaskrgr
Copy link
Member

Just an idea:

Let's say our code looks like this:

fn main() {
	{
		{
			{
				println!("Hello, world!");
		}
	}
}

The error message is not very helpful here, pointing to lines 1 and 8:

error: this file contains an un-closed delimiter
 --> src/main.rs:8:3
  |
8 | }
  |   ^
  |
help: did you mean to close this delimiter?
 --> src/main.rs:1:11
  |
1 | fn main() {
  |           ^

However, since cargo inits new repos as git repo, maybe we can utilize the repo diff and see which brace was added the last:

 fn main() {
        {
                {
-                       println!("Hello, world!");
+                       {
+                               println!("Hello, world!");
                }
        }
 }

and give a better diagnostic:

error: this file contains an un-closed delimiter
 --> src/main.rs:8:3
  |
8 | }
  |   ^
  |
help: did you mean to close this delimiter?
 --> src/main.rs:4:12
  |
4 |                 {
  |                 ^
```
@kennytm kennytm added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints labels Mar 10, 2018
@zackmdavis
Copy link
Member

Not worth the extra complexity, I would argue: teaching rustc to parse and interpret Git diffs would be just too large of dependency relative to the benefit of being able to provide a smarter error message in one particular case if the user happens to be using Git and the working tree has few enough changes from the last commit for us to be able to infer intent.

@Songbird0
Copy link
Contributor

[...] interpret Git diffs would be just too large of dependency relative to the benefit of being able to provide a smarter error message in one particular case if the user happens to be using Git and the working tree has few enough changes from the last commit for us to be able to infer intent.

Maybe add this feature as "optional" analytic process? Unless, of course, there aren't other scenarios where this may be useful.

@Ixrec
Copy link
Contributor

Ixrec commented Mar 15, 2018

For the specific case of curly brace matching errors, I think the indentation of the braces would be a far better heuristic for user intent than git history. The reason the opening brace on line 4 is "obviously" the unmatched one is because no other curly brace in the example has the same indentation. I'm not sure how to generalize that rule so it works on non-trivial examples, but it seems solvable.

(personally, it's not obvious to me that there are any situations where rustc looking at git history could lead to a significantly better error message, much less enough situations to justify the dependency)

@estebank
Copy link
Contributor

#53949 analyses the indentation of unmatched braces to provide appropriate suggestions and #54029 will provide further suggestions. I find that relying on the version control system to supply suggestions is an intriguing proposition, but unlikely to be pursued in the medium term.

@tromey
Copy link
Contributor

tromey commented Sep 14, 2018

One related thing that gcc and clang do is notice conflict markers from patch and emit a nicer error in that situation, like:

test.c:3:1: error: version control conflict marker in file

@zackmdavis
Copy link
Member

notice conflict markers from patch and emit a nicer error in that situation

(This is #32059.)

@estebank
Copy link
Contributor

Closing as #53949 addresses the mismatched braces issue for reasonably formatted code (and there's already another ticket for version control conflict markers). We're unlikely to ever ship with an implementation of git in the compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

7 participants