From 2ef93869428fcf19b3b374b23795fe559a045bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 5 May 2021 21:32:29 +0100 Subject: [PATCH] use an empty filename when re-parsing source files In printFile, we print and re-parse the modified AST to be able to have reliable position information. The re-parsing step can fail if something goes very wrong, such as a bug in -literals. It should generally not happen. However, in rare cases it has happened, and it's confusing for the end user to see syntax errors pointing at an existing file on disk, when the code doesn't align - since we're on a modified copy. To prevent such confusion, use an empty filename. Syntax errors will still not be terribly helpful, but they should be extremely rare and promptly fixed, so that's not a huge concern. For that same reason, we can't really add a good test here. We could perhaps add a test that forces garble to mess up the src slice in some way, but that would be a weird test, and not particularly worth it. Fixes #286. --- position.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/position.go b/position.go index 76eea90..d55324c 100644 --- a/position.go +++ b/position.go @@ -49,7 +49,12 @@ func printFile(file1 *ast.File) ([]byte, error) { // and those are the only source of truth that go/printer uses. // So the positions of the comments in the given file are wrong. // The only way we can get the final ones is to parse again. - file2, err := parser.ParseFile(fset, absFilename, src, parser.ParseComments) + // + // We use an empty filename here. + // Syntax errors should be rare, and when they do happen, + // we don't want to point to the original source file on disk. + // That would be confusing, as we've changed the source in memory. + file2, err := parser.ParseFile(fset, "", src, parser.ParseComments) if err != nil { return nil, fmt.Errorf("re-parse error: %w", err) }