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.

Sign up for GitHub

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

Jump to bottom

go/printer: form feed is wrongly treated as a newline #69858

Open
mateusz834 opened this issue Oct 12, 2024 · 3 comments · May be fixed by #69859
Open

go/printer: form feed is wrongly treated as a newline #69858

mateusz834 opened this issue Oct 12, 2024 · 3 comments · May be fixed by #69859
Assignees
@mateusz834
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@mateusz834
Copy link
Member

This is an unrealistic edge-case, found by a fuzzer.

Both of the go sources in the reproducer below, are parsed successfully, but the printed source (with go/printer) contains a syntax error:

func TestReproducer(t *testing.T) {
	cases := []string{
		"package A\nimport(\"a\"/*\f*/\n\"bb\")",
		"package A\nfunc test() {\"a\"/*\f*/\n\"bb\"}",
	}
	for _, src := range cases {
		fset := token.NewFileSet()
		f, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments|parser.SkipObjectResolution)
		if err != nil {
			t.Fatal(err)
		}

		ast.Print(fset, f)

		var out strings.Builder
		if err := Fprint(&out, fset, f); err != nil {
			t.Fatal(err)
		}

		_, err = parser.ParseFile(fset, "test.go", out.String(), parser.ParseComments|parser.SkipObjectResolution)
		if err != nil {
			t.Logf("source:\n%s\nformatted as:\n%s", src, out.String())
			t.Error(err) // test.go:4:11: expected ';', found "bb" (and 1 more errors)
		}
	}
}
[mateusz@arch go (master)]$ go test -run NotNewl -v go/printer -v
=== RUN   TestFormFeedNotNewline
    printer_test.go:887: source:
        package A
        import("a"/*
                    */
        "bb")
        formatted as:
        package A

        import (
                "a"     /*
                          */"bb"
        )
    printer_test.go:888: test.go:4:11: expected ';', found "bb" (and 1 more errors)
    printer_test.go:887: source:
        package A
        func test() {"a"/*
                          */
        "bb"}
        formatted as:
        package A

        func test() {
                "a"     /*
                          */"bb"
        }
    printer_test.go:888: test.go:4:11: expected ';', found "bb" (and 1 more errors)

This issue happens in the (*printer).print method.

go/src/go/printer/printer.go

Lines 1006 to 1020 in b521ebb

if !p.impliedSemi {
n := nlimit(next.Line - p.pos.Line)
// don't exceed maxNewlines if we already wrote one
if wroteNewline && n == maxNewlines {
n = maxNewlines - 1
}
if n > 0 {
ch := byte('\n')
if droppedFF {
ch = '\f' // use formfeed since we dropped one before
}
p.writeByte(ch, n)
impliedSemi = false
}
}

specifically here:

go/src/go/printer/printer.go

Line 1007 in b521ebb

n := nlimit(next.Line - p.pos.Line)

next.Line is equal to 3 and p.pos.Line is also 3, which causes zero newlines to be inserted, this happens because the (*printer).writeString method treats \f as a newline and incorrectly increases the p.pos.Line field.

go/src/go/printer/printer.go

Lines 312 to 334 in b521ebb

for i := 0; i < len(s); i++ {
// Raw string literals may contain any character except back quote (`).
if ch := s[i]; ch == '\n' || ch == '\f' {
// account for line break
nlines++
li = i
// A line break inside a literal will break whatever column
// formatting is in place; ignore any further alignment through
// the end of the line.
p.endAlignment = true
}
}
p.pos.Offset += len(s)
if nlines > 0 {
p.pos.Line += nlines
p.out.Line += nlines
c := len(s) - li
p.pos.Column = c
p.out.Column = c
} else {
p.pos.Column += len(s)
p.out.Column += len(s)
}

This was a nightmare to debug, but it seems like a fix is a one-liner, will send a CL.
@mateusz834 mateusz834 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 12, 2024
@mateusz834 mateusz834 self-assigned this Oct 12, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@mateusz834
Copy link
Member Author

CC @griesemer @adonovan

@mateusz834 mateusz834 linked a pull request Oct 12, 2024 that will close this issue
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/619499 mentions this issue: go/printer: do not treat '\f' as a newline in (*printer).writeString

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mateusz834 mateusz834

Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

go/printer: do not treat '\f' as a newline in (*printer).writeString mateusz834/go
3 participants
@gopherbot @mateusz834 @gabyhelp