Prevent blank lines above an item from moving with the item.

PiperOrigin-RevId: 533467581

parser/parser.go

@@ -567,25 +567,46 @@ func (p *parser) parse(isRoot bool) (result []*ast.Node, endPos ast.Position, er
 			return nil, ast.Position{}, err
 		}
 
-		startPos := p.position()
 		if p.nextInputIs('\n') {
 			// p.parse is often invoked with the index pointing at the
 			// newline character after the previous item.
 			// We should still report that this item starts in the next line.
-			startPos.Byte++
-			startPos.Line++
-			startPos.Column = 1
+			p.consume('\n')
 		}
+		startPos := p.position()
 
 		// Read PreComments.
 		comments, blankLines := p.skipWhiteSpaceAndReadComments(true /* multiLine */)
 
 		// Handle blank lines.
-		if blankLines > 1 {
+		if blankLines > 0 {
 			if p.config.infoLevel() {
 				p.config.infof("blankLines: %v", blankLines)
+				p.config.infof("comments: %v", comments)
+			}
+			if len(res) > 0 && res[len(res)-1].IsCommentOnly() {
+				// Blanks lines after a comment-only node are collapsed into one blank
+				// line which becomes an independent unnamed node, so that the blank
+				// line does not move when the item node below it moves for sorting.
+				//
+				// This allows users to add a blank line after a header comment as a way
+				// to avoid the header comment moving if the first item moves due to
+				// sorting.
+				res = append(res, &ast.Node{Start: startPos, PreComments: []string{""}})
+				// Each blank line is exactly 1 byte, and we need to adjust the start
+				// position for the next node to be after all the collapsed blank lines
+				// represented by the just added node.
+				startPos.Byte += uint32(blankLines)
+				startPos.Line += int32(blankLines)
+				startPos.Column = 1
+			} else {
+				// Unless the previous node is a comment-only node, we collapse the
+				// blanks lines into one blank line as the start of the comment (if any)
+				// of the next node, such that this blank line will move with the node
+				// when sorting, will be deleted with the node if targeted for deletion,
+				// etc.
+				comments = append([]string{""}, comments...)
 			}
-			comments = append([]string{""}, comments...)
 		}
 
 		for p.nextInputIs('%') {
@@ -824,15 +845,20 @@ func (p *parser) readContinuousBlocksOfComments() []string {
 // skipWhiteSpaceAndReadComments has multiple cases:
 //   - (1) reading a block of comments followed by a blank line
 //   - (2) reading a block of comments followed by non-blank content
-//   - (3) reading the inline comments between the current char and the end of the
-//     current line
+//   - (3) reading the inline comments between the current char and the end of
+//     the current line
+//
+// In both cases (1) and (2), there can also be blank lines before the comment
+// starts.
 //
-// Lines of comments and number of blank lines will be returned.
+// Lines of comments and number of blank lines before the comment will be
+// returned. If there is no comment, the returned slice will be empty.
 func (p *parser) skipWhiteSpaceAndReadComments(multiLine bool) ([]string, int) {
 	i := p.index
 	var foundComment, insideComment bool
 	commentBegin := 0
 	var comments []string
+	// Number of blanks lines *before* the comment (if any) starts.
 	blankLines := 0
 	for ; i < p.length; i++ {
 		if p.in[i] == '#' && !insideComment {

parser/parser_position_test.go

@@ -58,7 +58,7 @@ func TestParsePositions(t *testing.T) {
 			in: mkString(
 				"# top",        // 5 bytes + newline
 				"unrelated: 1", // 12 bytes + newline
-				"",             // this already gets accounted for trackme
+				"",             // this already gets accounted for trackme (blank line without a comment before it)
 				"trackme: 4",
 			),
 			startByte: 19,
@@ -76,12 +76,23 @@ func TestParsePositions(t *testing.T) {
 			in: mkString(
 				"# top",        // 5 bytes + newline
 				"unrelated: 1", // 12 bytes + newline
-				"",             // this already gets accounted for trackme
+				"",             // this already gets accounted for trackme (blank line without a comment before it)
 				"# boo",
 				"trackme: 4",
 			),
 			startByte: 19,
 		},
+		{
+			in: mkString(
+				"# top",              // 5 bytes + newline
+				"unrelated: 1",       // 12 bytes + newline
+				"",                   // newline
+				"# detached comment", // 18 bytes + newline
+				"",                   // newline (blank line with a comment before it becomes an unnamed node, not attached to trackme)
+				"trackme: 4",
+			),
+			startByte: 40,
+		},
 		{
 			in: mkString(
 				"outer: {", // 8 bytes + newline

parser/parser_test.go

@@ -1126,6 +1126,57 @@ presubmit: {
     prohibited_regexp: "UnsafeFunction"
   }
 }
+`}, {
+		name: "blank lines before comment blocks are collapsed to one",
+		in: `# txtpbfmt: sort_repeated_fields_by_content
+presubmit: {
+  check_contents: {
+
+
+    # Should go after ADD.
+    # And the empty lines above this are collapsed into one blank line.
+    operation: EDIT
+    operation: ADD
+
+
+
+    operation: REMOVE
+  }
+}
+`,
+		out: `# txtpbfmt: sort_repeated_fields_by_content
+presubmit: {
+  check_contents: {
+    operation: ADD
+
+    # Should go after ADD.
+    # And the empty lines above this are collapsed into one blank line.
+    operation: EDIT
+
+    operation: REMOVE
+  }
+}
+`}, {
+		name: "blank lines before comment blocks are collapsed to one",
+		in: `# txtpbfmt: sort_repeated_fields_by_content
+presubmit: {
+  check_contents: {
+    # This comment is a separate node and does not move when the fields are sorted.
+
+    operation: EDIT
+    operation: ADD
+  }
+}
+`,
+		out: `# txtpbfmt: sort_repeated_fields_by_content
+presubmit: {
+  check_contents: {
+    # This comment is a separate node and does not move when the fields are sorted.
+
+    operation: ADD
+    operation: EDIT
+  }
+}
 `}, {
 		name: "sort by subfield values",
 		in: `# txtpbfmt: sort_repeated_fields_by_subfield=operation.name
@@ -1301,23 +1352,50 @@ message: { id: "a" }
 		out: `# txtpbfmt: sort_repeated_fields_by_content
 # txtpbfmt: sort_repeated_fields_by_subfield=id
 
+
 # field a
 field: "a"
-
 # field b
 field: "b"
 message: { id: "a" }
 message: { id: "b" }
 
 # new group
 
+
 # field a
 field: "a"
-
 # field b
 field: "b"
 message: { id: "a" }
 message: { id: "b" }
+`}, {
+		name: "detached comment creates a new group for sorting",
+		in: `# txtpbfmt: sort_repeated_fields_by_content
+
+# field c
+field: "c"
+
+# field a
+field: "a"
+
+# new group - the fields below don't get sorted with the fields above
+
+# field b
+field: "b"
+`,
+		out: `# txtpbfmt: sort_repeated_fields_by_content
+
+
+# field a
+field: "a"
+# field c
+field: "c"
+
+# new group - the fields below don't get sorted with the fields above
+
+# field b
+field: "b"
 `}, {
 		name: "trailing comma / semicolon",
 		in: `dict: {
@@ -1516,6 +1594,61 @@ func TestParserConfigs(t *testing.T) {
   # Should remain below
   auto_reviewers: "reviewerA"
 }
+`,
+	}, {
+		name: "SortRepeatedFieldsBySubfieldWithHeaderComment",
+		in: `# proto-file: path/to/my/file.proto
+# proto-message: my.package.MyMessage
+#
+# My comments here.
+foo: {
+  bar: EDIT
+}
+foo: {
+  bar: ADD
+}
+`,
+		config: Config{SortRepeatedFieldsBySubfield: []string{"bar"}},
+		// We always attach the comment to the item - there is no special support
+		// for header comments. See the EmptyLineDetachesComment test case below for
+		// a workaround.
+		out: `foo: {
+  bar: ADD
+}
+# proto-file: path/to/my/file.proto
+# proto-message: my.package.MyMessage
+#
+# My comments here.
+foo: {
+  bar: EDIT
+}
+`,
+	}, {
+		name: "SortRepeatedFieldsBySubfieldWithHeaderComment_EmptyLineDetachesComment",
+		in: `# proto-file: path/to/my/file.proto
+# proto-message: my.package.MyMessage
+#
+# My comments here.
+
+foo: {
+  bar: EDIT
+}
+foo: {
+  bar: ADD
+}
+`,
+		config: Config{SortRepeatedFieldsBySubfield: []string{"bar"}},
+		out: `# proto-file: path/to/my/file.proto
+# proto-message: my.package.MyMessage
+#
+# My comments here.
+
+foo: {
+  bar: ADD
+}
+foo: {
+  bar: EDIT
+}
 `,
 	}, {
 		name: "SortNamedFieldByMultipleSubfieldContents",