From a0f8f33b629a2bfc84504b5859ca9dad876c7b59 Mon Sep 17 00:00:00 2001
From: elenad <elenad@vmware.com>
Date: Wed, 31 Mar 2021 08:07:45 -0700
Subject: [PATCH 01/32] Add testcases

---
 build/testdata/067.golden | 4 ++++
 build/testdata/067.in     | 1 +
 build/testdata/068.golden | 6 ++++++
 build/testdata/068.in     | 2 ++
 build/testdata/069.golden | 6 ++++++
 build/testdata/069.in     | 2 ++
 build/testdata/070.golden | 6 ++++++
 build/testdata/070.in     | 2 ++
 build/testdata/071.golden | 6 ++++++
 build/testdata/071.in     | 6 ++++++
 build/testdata/072.golden | 3 +++
 build/testdata/072.in     | 5 +++++
 build/testdata/073.golden | 6 ++++++
 build/testdata/073.in     | 2 ++
 14 files changed, 57 insertions(+)
 create mode 100644 build/testdata/067.golden
 create mode 100644 build/testdata/067.in
 create mode 100644 build/testdata/068.golden
 create mode 100644 build/testdata/068.in
 create mode 100644 build/testdata/069.golden
 create mode 100644 build/testdata/069.in
 create mode 100644 build/testdata/070.golden
 create mode 100644 build/testdata/070.in
 create mode 100644 build/testdata/071.golden
 create mode 100644 build/testdata/071.in
 create mode 100644 build/testdata/072.golden
 create mode 100644 build/testdata/072.in
 create mode 100644 build/testdata/073.golden
 create mode 100644 build/testdata/073.in

diff --git a/build/testdata/067.golden b/build/testdata/067.golden
new file mode 100644
index 000000000..ea1a67800
--- /dev/null
+++ b/build/testdata/067.golden
@@ -0,0 +1,4 @@
+manifest = [
+    ("a", "b", "c"),
+    ("ab", "bc", "cd"),
+]
diff --git a/build/testdata/067.in b/build/testdata/067.in
new file mode 100644
index 000000000..6404b2cab
--- /dev/null
+++ b/build/testdata/067.in
@@ -0,0 +1 @@
+manifest = [("a",  "b",  "c"), ("ab", "bc", "cd"),]
diff --git a/build/testdata/068.golden b/build/testdata/068.golden
new file mode 100644
index 000000000..ee5e66069
--- /dev/null
+++ b/build/testdata/068.golden
@@ -0,0 +1,6 @@
+# buildifier: table sort 1
+manifest = [
+   ("a",  "b",  "c"),
+   ("ab", "bc", "cd"),
+   ("",   "",    ""),
+]
diff --git a/build/testdata/068.in b/build/testdata/068.in
new file mode 100644
index 000000000..82226cd9f
--- /dev/null
+++ b/build/testdata/068.in
@@ -0,0 +1,2 @@
+# buildifier: table sort 1
+manifest = [("ab", "bc", "cd"), ("a", "b", "c"), ("", "", ""),]
diff --git a/build/testdata/069.golden b/build/testdata/069.golden
new file mode 100644
index 000000000..1d20875ab
--- /dev/null
+++ b/build/testdata/069.golden
@@ -0,0 +1,6 @@
+# buildifier: table nosort
+manifest = [
+   ("a",  "b",  "c"),
+   ("ab", "bc", "cd"),
+   ("",   "",   "")
+]
diff --git a/build/testdata/069.in b/build/testdata/069.in
new file mode 100644
index 000000000..55b9252cb
--- /dev/null
+++ b/build/testdata/069.in
@@ -0,0 +1,2 @@
+# buildifier: table nosort
+manifest = [("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
diff --git a/build/testdata/070.golden b/build/testdata/070.golden
new file mode 100644
index 000000000..1e29f0161
--- /dev/null
+++ b/build/testdata/070.golden
@@ -0,0 +1,6 @@
+# buildifier: table nosort
+manifest = [
+   ("",   "",   ""),
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c")
+]
diff --git a/build/testdata/070.in b/build/testdata/070.in
new file mode 100644
index 000000000..77e2958a3
--- /dev/null
+++ b/build/testdata/070.in
@@ -0,0 +1,2 @@
+# buildifier: table nosort
+manifest = [                   ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
diff --git a/build/testdata/071.golden b/build/testdata/071.golden
new file mode 100644
index 000000000..df76e3d91
--- /dev/null
+++ b/build/testdata/071.golden
@@ -0,0 +1,6 @@
+# buildifier: table nosort
+manifest = [
+    ("",   "",   ""),
+    ("ab", "bc", "cd"),
+    ("a",  "b",  "c"),
+]
diff --git a/build/testdata/071.in b/build/testdata/071.in
new file mode 100644
index 000000000..77631b36f
--- /dev/null
+++ b/build/testdata/071.in
@@ -0,0 +1,6 @@
+# buildifier: table nosort
+manifest = [
+    ("", "", ""),
+    ("ab", "bc", "cd"),
+    ("a", "b", "c"),
+]
diff --git a/build/testdata/072.golden b/build/testdata/072.golden
new file mode 100644
index 000000000..fb8ce6295
--- /dev/null
+++ b/build/testdata/072.golden
@@ -0,0 +1,3 @@
+# buildifier: table nosort
+manifest = [
+]
diff --git a/build/testdata/072.in b/build/testdata/072.in
new file mode 100644
index 000000000..9fecab6e7
--- /dev/null
+++ b/build/testdata/072.in
@@ -0,0 +1,5 @@
+# buildifier: table nosort
+manifest = [
+
+
+]
diff --git a/build/testdata/073.golden b/build/testdata/073.golden
new file mode 100644
index 000000000..4ee718bed
--- /dev/null
+++ b/build/testdata/073.golden
@@ -0,0 +1,6 @@
+# buildifier: table nosort
+manifest = [
+   (),
+   (),
+   ()
+]
diff --git a/build/testdata/073.in b/build/testdata/073.in
new file mode 100644
index 000000000..c46f4d102
--- /dev/null
+++ b/build/testdata/073.in
@@ -0,0 +1,2 @@
+# buildifier: table nosort
+manifest = [(), (), ()]

From 5aeb63dd4c967b56e67c1e4147ae0b57aa361f21 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Tue, 30 Mar 2021 19:53:04 -0700
Subject: [PATCH 02/32] TODO revert : Test commit

---
 README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/README.md b/README.md
index 0f5be17f1..afeb15a1b 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,4 @@
+# Testing commit
 # Buildtools for bazel
 
 This repository contains developer tools for working with Google's `bazel` buildtool.

From d59c7f5926dd7ee21411ef3877e8469bcad81e29 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Tue, 30 Mar 2021 19:55:47 -0700
Subject: [PATCH 03/32] Revert last test commit

---
 README.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/README.md b/README.md
index afeb15a1b..0f5be17f1 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,3 @@
-# Testing commit
 # Buildtools for bazel
 
 This repository contains developer tools for working with Google's `bazel` buildtool.

From 3700471bab441dfd574f15d71933712ac17f38d2 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Wed, 31 Mar 2021 21:30:40 +0530
Subject: [PATCH 04/32] Introducing Tabwriter in print.go

---
 build/print.go | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/build/print.go b/build/print.go
index 1df35a123..2ab0f6f85 100644
--- a/build/print.go
+++ b/build/print.go
@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"fmt"
 	"strings"
+	"text/tabwriter"
 )
 
 const (
@@ -63,18 +64,26 @@ func FormatString(x Expr) string {
 
 // A printer collects the state during printing of a file or expression.
 type printer struct {
-	fileType     FileType  // different rules can be applied to different file types.
-	bytes.Buffer           // output buffer
-	comment      []Comment // pending end-of-line comments
-	margin       int       // left margin (indent), a number of spaces
-	depth        int       // nesting depth inside ( ) [ ] { }
-	level        int       // nesting level of def-, if-else- and for-blocks
-	needsNewLine bool      // true if the next statement needs a new line before it
+	fileType     FileType          // different rules can be applied to different file types.
+	bytes.Buffer                   // output buffer
+	comment      []Comment         // pending end-of-line comments
+	margin       int               // left margin (indent), a number of spaces
+	depth        int               // nesting depth inside ( ) [ ] { }
+	level        int               // nesting level of def-, if-else- and for-blocks
+	needsNewLine bool              // true if the next statement needs a new line before it
+	tabWriterOn  bool              // when this mode is on , use the tabwriter to write to buffer
+	tw           *tabwriter.Writer // tab writer
 }
 
 // printf prints to the buffer.
 func (p *printer) printf(format string, args ...interface{}) {
-	fmt.Fprintf(p, format, args...)
+	if !p.tabWriterOn {
+		fmt.Fprintf(p, format, args...)
+	} else {
+		if p.tw != nil {
+			fmt.Fprintf(p.tw, format, args...)
+		}
+	}
 }
 
 // indent returns the position on the current line, in bytes, 0-indexed.
@@ -158,6 +167,11 @@ func (p *printer) trim() {
 
 // file formats the given file into the print buffer.
 func (p *printer) file(f *File) {
+	p.tabWriterOn = true
+	if p.tabWriterOn {
+		p.tw = new(tabwriter.Writer)
+		p.tw.Init(p, 0, 0, 4, ' ', tabwriter.StripEscape)
+	}
 	for _, com := range f.Before {
 		p.printf("%s", strings.TrimSpace(com.Token))
 		p.newline()
@@ -832,7 +846,11 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 	if p.useCompactMode(start, list, end, mode, forceCompact, forceMultiLine) {
 		for i, x := range *list {
 			if i > 0 {
-				p.printf(", ")
+				if !p.tabWriterOn {
+					p.printf(", ")
+				} else {
+					p.printf(",\t")
+				}
 			}
 			p.expr(x, precLow)
 		}

From 90d6ff0aba217835ee47b4c8a4e840bf7c875640 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Wed, 31 Mar 2021 23:11:50 +0530
Subject: [PATCH 05/32] TabWriter - Version 2

---
 build/print.go  | 35 +++++++++++++++++++++++++----------
 build/syntax.go |  6 ++++--
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/build/print.go b/build/print.go
index 2ab0f6f85..bfdacf5c1 100644
--- a/build/print.go
+++ b/build/print.go
@@ -167,11 +167,6 @@ func (p *printer) trim() {
 
 // file formats the given file into the print buffer.
 func (p *printer) file(f *File) {
-	p.tabWriterOn = true
-	if p.tabWriterOn {
-		p.tw = new(tabwriter.Writer)
-		p.tw.Init(p, 0, 0, 4, ' ', tabwriter.StripEscape)
-	}
 	for _, com := range f.Before {
 		p.printf("%s", strings.TrimSpace(com.Token))
 		p.newline()
@@ -621,7 +616,14 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false)
 
 	case *ListExpr:
+		// TODO: Check whether tabular mode is necessary.
+		p.tabWriterOn = true
+		if p.tabWriterOn {
+			p.tw = new(tabwriter.Writer)
+			p.tw.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
+		}
 		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
+		p.tw.Flush()
 
 	case *SetExpr:
 		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
@@ -631,6 +633,8 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		if v.NoBrackets {
 			mode = modeSeq
 		}
+		// TODO: Set this on a criteria
+		mode = modeTable
 		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine)
 
 	case *DictExpr:
@@ -761,6 +765,7 @@ const (
 	modeSeq   // x, y
 	modeDef   // def f(x, y)
 	modeLoad  // load(a, b, c)
+	modeTable // [(a,    b,    c)]
 )
 
 // useCompactMode reports whether a sequence should be formatted in a compact mode
@@ -843,14 +848,24 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 		}
 	}()
 
+	if mode == modeTable {
+		for i, x := range *list {
+			if i > 0 {
+				p.printf(",\t")
+			}
+			p.expr(x, precLow)
+		}
+		// Single-element tuple must end with comma, to mark it as a tuple.
+		if len(*list) == 1 && mode == modeTuple {
+			p.printf(",")
+		}
+		return
+	}
+
 	if p.useCompactMode(start, list, end, mode, forceCompact, forceMultiLine) {
 		for i, x := range *list {
 			if i > 0 {
-				if !p.tabWriterOn {
-					p.printf(", ")
-				} else {
-					p.printf(",\t")
-				}
+				p.printf(", ")
 			}
 			p.expr(x, precLow)
 		}
diff --git a/build/syntax.go b/build/syntax.go
index 1e5b82a1a..4b5879701 100644
--- a/build/syntax.go
+++ b/build/syntax.go
@@ -194,8 +194,8 @@ func (x *Ident) asString() *StringExpr {
 // An TypedIdent represents an identifier with type annotation: "foo: int".
 type TypedIdent struct {
 	Comments
-	Ident   *Ident
-	Type    Expr
+	Ident *Ident
+	Type  Expr
 }
 
 // Span returns the start and end positions of the node
@@ -445,6 +445,7 @@ type ListExpr struct {
 	List  []Expr
 	End
 	ForceMultiLine bool // force multiline form when printing
+	ForceTable     bool // force tablular fornmatting when printing
 }
 
 // Span returns the start and end positions of the node
@@ -487,6 +488,7 @@ type TupleExpr struct {
 	End
 	ForceCompact   bool // force compact (non-multiline) form when printing
 	ForceMultiLine bool // force multiline form when printing
+	ForceTableRow  bool // force tablular formatting when printing
 }
 
 // Span returns the start and end positions of the node

From 38e9ede3b3483feb17c59751ffbb64e97ccb6ec9 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Wed, 31 Mar 2021 11:48:59 -0700
Subject: [PATCH 06/32] Add a placeholder method for where the metaData on the
 List and Tuples are set to mark them as contenders for table formatting. This
 will later to set these data whenever a correct buidifier tag is parsed.

---
 build/rewrite.go | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/build/rewrite.go b/build/rewrite.go
index 471a9287f..b6df28474 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -84,6 +84,7 @@ var rewrites = []struct {
 	{"removeParens", removeParens, scopeBuild},
 	{"callsort", sortCallArgs, scopeBuild},
 	{"label", fixLabels, scopeBuild},
+	{"formatTables", formatTables, scopeBoth},
 	{"listsort", sortStringLists, scopeBoth},
 	{"multiplus", fixMultilinePlus, scopeBuild},
 	{"loadsort", sortAllLoadArgs, scopeBoth},
@@ -376,6 +377,29 @@ func (x namedArgs) Less(i, j int) bool {
 	return p.index < q.index
 }
 
+// Detects if a data is marked as tablular based on the tag `Buildifier: Table`
+// and sets the correct flags on the expression.
+// If the tags necessitate a sorting of the tabular data, then this takes care of inplace sorting.
+func formatTables(f *File) {
+	Walk(f, func(v Expr, stk []Expr) {
+		switch v := v.(type) {
+		// Tabular formatting is currently supported only for lists
+		case *ListExpr:
+			// set based on Rehana's condition
+			v.ForceTable = true
+
+			// Iterate within the items of the list ( tablerows)
+			for _, row := range v.List {
+				tupleRow, ok := row.(*TupleExpr)
+				if !ok {
+					continue
+				}
+				tupleRow.ForceTableRow = true
+			}
+		}
+	})
+}
+
 // sortStringLists sorts lists of string literals used as specific rule arguments.
 func sortStringLists(f *File) {
 	Walk(f, func(v Expr, stk []Expr) {

From 9e5a59c457a597f02e4076f60e6632034a028757 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 13:09:51 -0700
Subject: [PATCH 07/32] Add tags for table formatting and sorting: #buildifier:
 table sort 1 , #buildifier: table

---
 build/rewrite.go | 49 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index b6df28474..1b6a6dd1f 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/bazelbuild/buildtools/tables"
@@ -118,6 +119,32 @@ func hasComment(x Expr, text string) bool {
 	return false
 }
 
+// getColumnNumber reports the column number specified in the x for
+// "buildifier: table sort <n>" in comment.
+func getColumnNumber(x Expr) int {
+	for _, com := range x.Comment().Before {
+		tokens := strings.Split(strings.ToLower(com.Token), " ")
+		colNum, _ := strconv.Atoi(tokens[4])
+		return colNum
+	}
+	return -1
+}
+
+// tableFormat reports whether x is marked with a comment containing
+// "buildifier: table", case-insensitive.
+func tableFormat(x Expr) bool {
+	return hasComment(x, "buildifier: table")
+}
+
+// tableFormatSortBy reports whether x is marked with a comment containing
+// "buildifier: table sort <n>", case-insensitive.
+func tableFormatSortBy(x Expr) int {
+	if hasComment(x, "buildifier: table sort") {
+		return getColumnNumber(x)
+	}
+	return -1
+}
+
 // leaveAlone1 reports whether x is marked with a comment containing
 // "buildifier: leave-alone", case-insensitive.
 func leaveAlone1(x Expr) bool {
@@ -385,16 +412,18 @@ func formatTables(f *File) {
 		switch v := v.(type) {
 		// Tabular formatting is currently supported only for lists
 		case *ListExpr:
-			// set based on Rehana's condition
-			v.ForceTable = true
+			if tableFormat(v.List[0]) {
 
-			// Iterate within the items of the list ( tablerows)
-			for _, row := range v.List {
-				tupleRow, ok := row.(*TupleExpr)
-				if !ok {
-					continue
+				v.ForceTable = true
+
+				// Iterate within the items of the list ( tablerows)
+				for _, row := range v.List {
+					tupleRow, ok := row.(*TupleExpr)
+					if !ok {
+						continue
+					}
+					tupleRow.ForceTableRow = true
 				}
-				tupleRow.ForceTableRow = true
 			}
 		}
 	})
@@ -456,6 +485,10 @@ func sortStringLists(f *File) {
 				SortStringList(v.Value)
 			}
 		case *ListExpr:
+			// TODO remove
+			if tableFormatSortBy(v.List[0]) > 0 {
+				// colNumber = tableFormatSortBy(v.List[0])
+			}
 			if disabled("unsafesort") {
 				return
 			}

From 78308e498cea50959360f6ad6b28bfdf112acc44 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 13:12:58 -0700
Subject: [PATCH 08/32] Update tags in the UnitTests

---
 build/testdata/069.golden | 2 +-
 build/testdata/069.in     | 2 +-
 build/testdata/070.golden | 2 +-
 build/testdata/070.in     | 2 +-
 build/testdata/071.golden | 2 +-
 build/testdata/071.in     | 2 +-
 build/testdata/072.golden | 2 +-
 build/testdata/072.in     | 2 +-
 build/testdata/073.golden | 2 +-
 build/testdata/073.in     | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/build/testdata/069.golden b/build/testdata/069.golden
index 1d20875ab..640ece932 100644
--- a/build/testdata/069.golden
+++ b/build/testdata/069.golden
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
    ("a",  "b",  "c"),
    ("ab", "bc", "cd"),
diff --git a/build/testdata/069.in b/build/testdata/069.in
index 55b9252cb..cbfcfff67 100644
--- a/build/testdata/069.in
+++ b/build/testdata/069.in
@@ -1,2 +1,2 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
diff --git a/build/testdata/070.golden b/build/testdata/070.golden
index 1e29f0161..b9a0648be 100644
--- a/build/testdata/070.golden
+++ b/build/testdata/070.golden
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
    ("",   "",   ""),
    ("ab", "bc", "cd"),
diff --git a/build/testdata/070.in b/build/testdata/070.in
index 77e2958a3..84234ec2c 100644
--- a/build/testdata/070.in
+++ b/build/testdata/070.in
@@ -1,2 +1,2 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [                   ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
diff --git a/build/testdata/071.golden b/build/testdata/071.golden
index df76e3d91..b3ababe17 100644
--- a/build/testdata/071.golden
+++ b/build/testdata/071.golden
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
     ("",   "",   ""),
     ("ab", "bc", "cd"),
diff --git a/build/testdata/071.in b/build/testdata/071.in
index 77631b36f..2f7d3df47 100644
--- a/build/testdata/071.in
+++ b/build/testdata/071.in
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
     ("", "", ""),
     ("ab", "bc", "cd"),
diff --git a/build/testdata/072.golden b/build/testdata/072.golden
index fb8ce6295..fe30b4602 100644
--- a/build/testdata/072.golden
+++ b/build/testdata/072.golden
@@ -1,3 +1,3 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
 ]
diff --git a/build/testdata/072.in b/build/testdata/072.in
index 9fecab6e7..9d4097961 100644
--- a/build/testdata/072.in
+++ b/build/testdata/072.in
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
 
 
diff --git a/build/testdata/073.golden b/build/testdata/073.golden
index 4ee718bed..673a42073 100644
--- a/build/testdata/073.golden
+++ b/build/testdata/073.golden
@@ -1,4 +1,4 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [
    (),
    (),
diff --git a/build/testdata/073.in b/build/testdata/073.in
index c46f4d102..f05ac60f8 100644
--- a/build/testdata/073.in
+++ b/build/testdata/073.in
@@ -1,2 +1,2 @@
-# buildifier: table nosort
+# buildifier: table
 manifest = [(), (), ()]

From 1c1705e7343ecdb8c239c19299a38df60979171d Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Wed, 31 Mar 2021 13:18:53 -0700
Subject: [PATCH 09/32] Fix 6 failing tests. Solution : Hook up tabWrite logic
 only when we detect a buildifier tag.

INFO: Build completed, 1 test FAILED, 60 total actions
//api_proto:api.gen.pb.go_checkshtest                           (cached) PASSED in 0.5s
//build_proto:build.gen.pb.go_checkshtest                       (cached) PASSED in 0.4s
//deps_proto:deps.gen.pb.go_checkshtest                         (cached) PASSED in 0.9s
//extra_actions_base_proto:extra_actions_base.gen.pb.go_checkshtest (cached) PASSED in 0.8s
//labels:go_default_test                                        (cached) PASSED in 0.6s
//lang:tables.gen.go_checkshtest                                (cached) PASSED in 0.7s
//tables:go_default_test                                        (cached) PASSED in 1.1s
//wspace:go_default_test                                        (cached) PASSED in 0.6s
//buildifier:buildifier_integration_test                                 PASSED in 2.5s
//buildifier/npm:integration_test                                        PASSED in 0.5s
//buildifier/utils:go_default_test                                       PASSED in 0.3s
//buildozer:buildozer_test                                               PASSED in 8.2s
//buildozer/npm:integration_test                                         PASSED in 0.6s
//bzlenv:bzlenv_test                                                     PASSED in 0.4s
//bzlenv:go_default_test                                                 PASSED in 0.6s
//edit:go_default_test                                                   PASSED in 0.9s
//unused_deps:go_default_test                                            PASSED in 0.6s
//unused_deps:jar_manifest_test                                          PASSED in 0.5s
//warn:go_default_test                                                   PASSED in 1.1s
//warn/docs:go_default_test                                              PASSED in 0.4s
//build:go_default_test                                                  FAILED in 1.2s
  /private/var/tmp/_bazel_kshwetha/3198714a19bbf3371758516069d33ca0/execroot/com_github_bazelbuild_buildtools/bazel-out/darwin-fastbuild/testlogs/build/go_default_test
---
 build/print.go | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/build/print.go b/build/print.go
index bfdacf5c1..ec1c10404 100644
--- a/build/print.go
+++ b/build/print.go
@@ -71,7 +71,7 @@ type printer struct {
 	depth        int               // nesting depth inside ( ) [ ] { }
 	level        int               // nesting level of def-, if-else- and for-blocks
 	needsNewLine bool              // true if the next statement needs a new line before it
-	tabWriterOn  bool              // when this mode is on , use the tabwriter to write to buffer
+	tabWriterOn  bool              // when this mode is on ,use the tabwriter to write to buffer
 	tw           *tabwriter.Writer // tab writer
 }
 
@@ -616,14 +616,17 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false)
 
 	case *ListExpr:
-		// TODO: Check whether tabular mode is necessary.
-		p.tabWriterOn = true
-		if p.tabWriterOn {
+		if v.ForceTable {
+			p.tabWriterOn = true
 			p.tw = new(tabwriter.Writer)
 			p.tw.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
 		}
+
 		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
-		p.tw.Flush()
+
+		if v.ForceTable {
+			p.tw.Flush()
+		}
 
 	case *SetExpr:
 		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
@@ -633,8 +636,9 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		if v.NoBrackets {
 			mode = modeSeq
 		}
-		// TODO: Set this on a criteria
-		mode = modeTable
+		if v.ForceTableRow {
+			mode = modeTable
+		}
 		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine)
 
 	case *DictExpr:

From 254df91cafe15f8d126c40b23d0a05106f5f93a0 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Wed, 31 Mar 2021 14:01:59 -0700
Subject: [PATCH 10/32] Fix Panics while reading tags - also test failures
 cause by it.

```
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

goroutine 16 [running]:
testing.tRunner.func1.1(0x11cd060, 0xc000524fe0)
	GOROOT/src/testing/testing.go:1072 +0x30d
testing.tRunner.func1(0xc000082900)
	GOROOT/src/testing/testing.go:1075 +0x41a
panic(0x11cd060, 0xc000524fe0)
	GOROOT/src/runtime/panic.go:969 +0x1b9
github.com/bazelbuild/buildtools/build.formatTables.func1(0x121b980, 0xc000198a80, 0xc0001a1e00, 0x3, 0x8)
	build/rewrite.go:415 +0xeb

goroutine 13 [running]:
testing.tRunner.func1.1(0x11cd060, 0xc0000add40)
	GOROOT/src/testing/testing.go:1072 +0x30d
testing.tRunner.func1(0xc000582300)
	GOROOT/src/testing/testing.go:1075 +0x41a
panic(0x11cd060, 0xc0000add40)
	GOROOT/src/runtime/panic.go:969 +0x1b9
github.com/bazelbuild/buildtools/build.sortStringLists.func1(0x121b980, 0xc000222620, 0xc000219b00, 0x3, 0x8)
	build/rewrite.go:488 +0x72f
github.com/bazelbuild/buildtools/build.Walk.func1(0xc000219530, 0xc000219b00, 0x3, 0x8, 0x0, 0x0)
	build/walk.go:28 +0x59
github.com/bazelbuild/buildtools/build.walk1(0xc000219530, 0xc00056fa48, 0xc00056fa38, 0x121bc00, 0xc000219480)
```
---
 build/rewrite.go | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index 1b6a6dd1f..bd5aee6af 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -412,8 +412,7 @@ func formatTables(f *File) {
 		switch v := v.(type) {
 		// Tabular formatting is currently supported only for lists
 		case *ListExpr:
-			if tableFormat(v.List[0]) {
-
+			if len(v.List) > 0 && tableFormat(v.List[0]) {
 				v.ForceTable = true
 
 				// Iterate within the items of the list ( tablerows)
@@ -485,10 +484,6 @@ func sortStringLists(f *File) {
 				SortStringList(v.Value)
 			}
 		case *ListExpr:
-			// TODO remove
-			if tableFormatSortBy(v.List[0]) > 0 {
-				// colNumber = tableFormatSortBy(v.List[0])
-			}
 			if disabled("unsafesort") {
 				return
 			}

From 6d157e1e706f3fad02a0f787dd255bbfaf12ca04 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 14:19:38 -0700
Subject: [PATCH 11/32] Update UnitTests to apply tags at the tabletop

---
 build/testdata/068.golden | 2 +-
 build/testdata/068.in     | 8 ++++++--
 build/testdata/069.golden | 2 +-
 build/testdata/069.in     | 4 ++--
 build/testdata/070.golden | 2 +-
 build/testdata/070.in     | 3 +--
 build/testdata/071.golden | 8 ++++----
 build/testdata/071.in     | 2 +-
 build/testdata/072.golden | 2 +-
 build/testdata/072.in     | 3 +--
 build/testdata/073.golden | 2 +-
 build/testdata/073.in     | 3 +--
 12 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/build/testdata/068.golden b/build/testdata/068.golden
index ee5e66069..82996e95a 100644
--- a/build/testdata/068.golden
+++ b/build/testdata/068.golden
@@ -1,5 +1,5 @@
-# buildifier: table sort 1
 manifest = [
+   # buildifier: table sort 1
    ("a",  "b",  "c"),
    ("ab", "bc", "cd"),
    ("",   "",    ""),
diff --git a/build/testdata/068.in b/build/testdata/068.in
index 82226cd9f..79d9de0d4 100644
--- a/build/testdata/068.in
+++ b/build/testdata/068.in
@@ -1,2 +1,6 @@
-# buildifier: table sort 1
-manifest = [("ab", "bc", "cd"), ("a", "b", "c"), ("", "", ""),]
+manifest = [
+   # buildifier: table sort 1
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c"),
+   ("",   "",    ""),
+]
\ No newline at end of file
diff --git a/build/testdata/069.golden b/build/testdata/069.golden
index 640ece932..0ce1e8709 100644
--- a/build/testdata/069.golden
+++ b/build/testdata/069.golden
@@ -1,5 +1,5 @@
-# buildifier: table
 manifest = [
+   # buildifier: table
    ("a",  "b",  "c"),
    ("ab", "bc", "cd"),
    ("",   "",   "")
diff --git a/build/testdata/069.in b/build/testdata/069.in
index cbfcfff67..827c14844 100644
--- a/build/testdata/069.in
+++ b/build/testdata/069.in
@@ -1,2 +1,2 @@
-# buildifier: table
-manifest = [("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
+manifest = [
+    # buildifier: table ("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
diff --git a/build/testdata/070.golden b/build/testdata/070.golden
index b9a0648be..ccfd39d0b 100644
--- a/build/testdata/070.golden
+++ b/build/testdata/070.golden
@@ -1,5 +1,5 @@
-# buildifier: table
 manifest = [
+   # buildifier: table
    ("",   "",   ""),
    ("ab", "bc", "cd"),
    ("a",  "b",  "c")
diff --git a/build/testdata/070.in b/build/testdata/070.in
index 84234ec2c..f77994a99 100644
--- a/build/testdata/070.in
+++ b/build/testdata/070.in
@@ -1,2 +1 @@
-# buildifier: table
-manifest = [                   ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
+manifest = [# buildifier: table                   ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
diff --git a/build/testdata/071.golden b/build/testdata/071.golden
index b3ababe17..4d302c377 100644
--- a/build/testdata/071.golden
+++ b/build/testdata/071.golden
@@ -1,6 +1,6 @@
-# buildifier: table
 manifest = [
-    ("",   "",   ""),
-    ("ab", "bc", "cd"),
-    ("a",  "b",  "c"),
+    # buildifier: table
+    ("",    "",     ""),
+    ("ab",  "bc",   "cd"),
+    ("a",   "b",    "c"),
 ]
diff --git a/build/testdata/071.in b/build/testdata/071.in
index 2f7d3df47..1712a1042 100644
--- a/build/testdata/071.in
+++ b/build/testdata/071.in
@@ -1,5 +1,5 @@
-# buildifier: table
 manifest = [
+    # buildifier: table
     ("", "", ""),
     ("ab", "bc", "cd"),
     ("a", "b", "c"),
diff --git a/build/testdata/072.golden b/build/testdata/072.golden
index fe30b4602..c7dee849a 100644
--- a/build/testdata/072.golden
+++ b/build/testdata/072.golden
@@ -1,3 +1,3 @@
-# buildifier: table
 manifest = [
+# buildifier: table
 ]
diff --git a/build/testdata/072.in b/build/testdata/072.in
index 9d4097961..29817ff04 100644
--- a/build/testdata/072.in
+++ b/build/testdata/072.in
@@ -1,5 +1,4 @@
-# buildifier: table
 manifest = [
-
+# buildifier: table
 
 ]
diff --git a/build/testdata/073.golden b/build/testdata/073.golden
index 673a42073..55f37734f 100644
--- a/build/testdata/073.golden
+++ b/build/testdata/073.golden
@@ -1,5 +1,5 @@
-# buildifier: table
 manifest = [
+   # buildifier: table
    (),
    (),
    ()
diff --git a/build/testdata/073.in b/build/testdata/073.in
index f05ac60f8..386b27732 100644
--- a/build/testdata/073.in
+++ b/build/testdata/073.in
@@ -1,2 +1 @@
-# buildifier: table
-manifest = [(), (), ()]
+manifest = [# buildifier: table (), (), ()]

From 4703c9d8faf2215561895dff11495999511018b7 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 14:33:46 -0700
Subject: [PATCH 12/32] Add more UnitTests for testcases with - long column
 width, variable column widths

---
 build/testdata/073.in     | 5 ++++-
 build/testdata/074.golden | 6 ++++++
 build/testdata/074.in     | 6 ++++++
 build/testdata/075.golden | 6 ++++++
 build/testdata/075.in     | 7 +++++++
 5 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 build/testdata/074.golden
 create mode 100644 build/testdata/074.in
 create mode 100644 build/testdata/075.golden
 create mode 100644 build/testdata/075.in

diff --git a/build/testdata/073.in b/build/testdata/073.in
index 386b27732..811975bd6 100644
--- a/build/testdata/073.in
+++ b/build/testdata/073.in
@@ -1 +1,4 @@
-manifest = [# buildifier: table (), (), ()]
+manifest = [
+    # buildifier: table 
+    (), (), ()
+]
diff --git a/build/testdata/074.golden b/build/testdata/074.golden
new file mode 100644
index 000000000..ce8c3266b
--- /dev/null
+++ b/build/testdata/074.golden
@@ -0,0 +1,6 @@
+manifest = [
+    # buildifier: table
+    ("",                        "",     ""),
+    ("ab",                      "bc",   "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa",    "b",    "c"),
+]
diff --git a/build/testdata/074.in b/build/testdata/074.in
new file mode 100644
index 000000000..21c4f95a3
--- /dev/null
+++ b/build/testdata/074.in
@@ -0,0 +1,6 @@
+manifest = [
+    # buildifier: table
+    ("", "", ""),
+    ("ab", "bc", "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa", "b", "c"),
+]
diff --git a/build/testdata/075.golden b/build/testdata/075.golden
new file mode 100644
index 000000000..2cd66fa34
--- /dev/null
+++ b/build/testdata/075.golden
@@ -0,0 +1,6 @@
+manifest = [
+    # buildifier: table
+    ("",                        "",                         ""),
+    ("ab",                      "bcbcbcbcbcbcbcbcbcbcbc",   "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa",    "b",                        "c"),
+]
diff --git a/build/testdata/075.in b/build/testdata/075.in
new file mode 100644
index 000000000..082edfaf1
--- /dev/null
+++ b/build/testdata/075.in
@@ -0,0 +1,7 @@
+manifest = [
+    # buildifier: table
+    ("", "", ""),
+    ("ab", "bcbcbcbcbcbcbcbcbcbcbc", "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa", "b", "c"),
+]
+

From bfbc5c4ba498b3f64dd52bb1bf9b56e42d39941d Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 16:58:54 -0700
Subject: [PATCH 13/32] Fix issue with table sort tag - panic: runtime error:
 index out of range [0] with length 0

---
 build/rewrite.go | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index bd5aee6af..382202ca5 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -136,9 +136,10 @@ func tableFormat(x Expr) bool {
 	return hasComment(x, "buildifier: table")
 }
 
-// tableFormatSortBy reports whether x is marked with a comment containing
-// "buildifier: table sort <n>", case-insensitive.
-func tableFormatSortBy(x Expr) int {
+// tableSort reports whether x is marked with a comment containing
+// "buildifier: table sort <n>", case-insensitive and returns the
+// column number.
+func tableSort(x Expr) int {
 	if hasComment(x, "buildifier: table sort") {
 		return getColumnNumber(x)
 	}
@@ -404,7 +405,7 @@ func (x namedArgs) Less(i, j int) bool {
 	return p.index < q.index
 }
 
-// Detects if a data is marked as tablular based on the tag `Buildifier: Table`
+// Detects if a data is marked as tablular based on the tag `buildifier: table`
 // and sets the correct flags on the expression.
 // If the tags necessitate a sorting of the tabular data, then this takes care of inplace sorting.
 func formatTables(f *File) {
@@ -484,6 +485,10 @@ func sortStringLists(f *File) {
 				SortStringList(v.Value)
 			}
 		case *ListExpr:
+			// TODO remove
+			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
+				// colNumber = tableSort(v.List[0])
+			}
 			if disabled("unsafesort") {
 				return
 			}

From f794bd2ea303f2c769c14d069054e801bf3aa1f9 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Wed, 31 Mar 2021 21:33:13 -0700
Subject: [PATCH 14/32] First version of Table sorting with tag #buildifier:
 table sort N

---
 build/rewrite.go | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index 382202ca5..e1a6c6b70 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -413,9 +413,9 @@ func formatTables(f *File) {
 		switch v := v.(type) {
 		// Tabular formatting is currently supported only for lists
 		case *ListExpr:
+			// Handle when "#buildifier: table" tag is set
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
 				v.ForceTable = true
-
 				// Iterate within the items of the list ( tablerows)
 				for _, row := range v.List {
 					tupleRow, ok := row.(*TupleExpr)
@@ -425,6 +425,40 @@ func formatTables(f *File) {
 					tupleRow.ForceTableRow = true
 				}
 			}
+			// Handle when "#buildifier: table sort N" tag is set
+			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
+				// less than 2 rows, nothing to sort
+				if len(v.List) < 2 {
+					return
+				}
+
+				var sortedList []Expr
+				colNumber := tableSort(v.List[0])
+				keys := make([]string, 0, len(v.List))
+				tableMap := make(map[string]*TupleExpr)
+				// Iterate  over the list of tuples(tablerows)
+				for _, row := range v.List {
+					tupleRow, ok := row.(*TupleExpr)
+					if !ok {
+						continue
+					}
+					// keys are the values from tuple[colNumber position]
+					// Table column starts at 1, where list starts at 0
+					str, ok := tupleRow.List[colNumber-1].(*StringExpr)
+					key := str.Value
+					keys = append(keys, key)
+					// create a map with <key, tuple> values
+					tableMap[key] = tupleRow
+				}
+
+				// sort the keys
+				sort.Strings(keys)
+				// rewrite the sorted list to v.list
+				for _, key := range keys {
+					sortedList = append(sortedList, tableMap[key])
+				}
+				v.List = sortedList
+			}
 		}
 	})
 }
@@ -485,10 +519,6 @@ func sortStringLists(f *File) {
 				SortStringList(v.Value)
 			}
 		case *ListExpr:
-			// TODO remove
-			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
-				// colNumber = tableSort(v.List[0])
-			}
 			if disabled("unsafesort") {
 				return
 			}

From f642928b1dbc676f4b37fa318702eb5c9ed4ec3f Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Thu, 1 Apr 2021 01:02:15 -0700
Subject: [PATCH 15/32] Attempt fix sort bug

---
 build/rewrite.go | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index e1a6c6b70..d337e08df 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -19,6 +19,7 @@ limitations under the License.
 package build
 
 import (
+	"fmt"
 	"path"
 	"path/filepath"
 	"regexp"
@@ -433,6 +434,7 @@ func formatTables(f *File) {
 				}
 
 				var sortedList []Expr
+				var comments []Comment
 				colNumber := tableSort(v.List[0])
 				keys := make([]string, 0, len(v.List))
 				tableMap := make(map[string]*TupleExpr)
@@ -447,6 +449,12 @@ func formatTables(f *File) {
 					str, ok := tupleRow.List[colNumber-1].(*StringExpr)
 					key := str.Value
 					keys = append(keys, key)
+
+					// This is a duplicate of a string above.
+					// Collect comments so that they're not lost.
+					comments = append(comments, tupleRow.Comment().Before...)
+					fmt.Println("RTRT", comments)
+
 					// create a map with <key, tuple> values
 					tableMap[key] = tupleRow
 				}
@@ -454,7 +462,10 @@ func formatTables(f *File) {
 				// sort the keys
 				sort.Strings(keys)
 				// rewrite the sorted list to v.list
-				for _, key := range keys {
+				for i, key := range keys {
+					if i == 0 {
+						tableMap[key].Comment().Before = comments
+					}
 					sortedList = append(sortedList, tableMap[key])
 				}
 				v.List = sortedList

From 49185cbef79572238a8ddd3a0cf0d9a0c2dce81b Mon Sep 17 00:00:00 2001
From: elenad <elenad@vmware.com>
Date: Thu, 1 Apr 2021 01:39:24 -0700
Subject: [PATCH 16/32] Fix sorting

---
 build/rewrite.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index d337e08df..5d2a2ce43 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -460,12 +460,19 @@ func formatTables(f *File) {
 				}
 
 				// sort the keys
+                                var CommentKey []Comment
+                                for i, key := range keys {
+                                        if i == 0 {
+                                                CommentKey = tableMap[key].Comment().Before[0:1]
+                                                tableMap[key].Comment().Before = tableMap[key].Comment().Before[1:]
+                                        }
+                                }
 				sort.Strings(keys)
 				// rewrite the sorted list to v.list
 				for i, key := range keys {
 					if i == 0 {
-						tableMap[key].Comment().Before = comments
-					}
+	          				tableMap[key].Comment().Before = CommentKey
+                                        }
 					sortedList = append(sortedList, tableMap[key])
 				}
 				v.List = sortedList

From 7dcad2b084caf79f7a5f6e070f0c4f1e8c0d6881 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Thu, 1 Apr 2021 14:16:31 +0530
Subject: [PATCH 17/32] Changes done - 1. Proper alignment of comments when
 tabwriter is on. 2. Revision of in and golden files.

---
 build/print.go            |  4 +++-
 build/testdata/067.golden |  5 +----
 build/testdata/067.in     |  2 +-
 build/testdata/068.golden | 10 +++++-----
 build/testdata/069.golden | 10 +++++-----
 build/testdata/069.in     |  3 ++-
 build/testdata/070.golden | 10 +++++-----
 build/testdata/070.in     |  4 +++-
 build/testdata/071.golden |  8 ++++----
 build/testdata/072.golden |  2 +-
 build/testdata/073.golden | 10 +++++-----
 build/testdata/074.golden |  8 ++++----
 build/testdata/075.golden |  8 ++++----
 13 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/build/print.go b/build/print.go
index ec1c10404..98f468da8 100644
--- a/build/print.go
+++ b/build/print.go
@@ -383,7 +383,9 @@ func (p *printer) expr(v Expr, outerPrec int) {
 			p.printf("\n")
 		}
 		// Re-indent to margin.
-		p.printf("%*s", p.margin, "")
+		if !p.tabWriterOn {
+			p.printf("%*s", p.margin, "")
+		}
 		for _, com := range before {
 			p.printf("%s", strings.TrimSpace(com.Token))
 			p.newline()
diff --git a/build/testdata/067.golden b/build/testdata/067.golden
index ea1a67800..b7e1f0bb4 100644
--- a/build/testdata/067.golden
+++ b/build/testdata/067.golden
@@ -1,4 +1 @@
-manifest = [
-    ("a", "b", "c"),
-    ("ab", "bc", "cd"),
-]
+manifest = [("a", "b", "c"), ("ab", "bc", "cd")]
\ No newline at end of file
diff --git a/build/testdata/067.in b/build/testdata/067.in
index 6404b2cab..73f85ac30 100644
--- a/build/testdata/067.in
+++ b/build/testdata/067.in
@@ -1 +1 @@
-manifest = [("a",  "b",  "c"), ("ab", "bc", "cd"),]
+manifest = [("a",  "b",  "c"), ("ab", "bc", "cd"),]
\ No newline at end of file
diff --git a/build/testdata/068.golden b/build/testdata/068.golden
index 82996e95a..582516d37 100644
--- a/build/testdata/068.golden
+++ b/build/testdata/068.golden
@@ -1,6 +1,6 @@
-manifest = [
-   # buildifier: table sort 1
-   ("a",  "b",  "c"),
-   ("ab", "bc", "cd"),
-   ("",   "",    ""),
+manifest =[
+    # buildifier: table sort 1
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
+    ("",      "",      ""),
 ]
diff --git a/build/testdata/069.golden b/build/testdata/069.golden
index 0ce1e8709..05799bdf7 100644
--- a/build/testdata/069.golden
+++ b/build/testdata/069.golden
@@ -1,6 +1,6 @@
-manifest = [
-   # buildifier: table
-   ("a",  "b",  "c"),
-   ("ab", "bc", "cd"),
-   ("",   "",   "")
+manifest =[
+    # buildifier: table
+    ("a",     "b",     "c"),
+    ("ab",    "bc",    "cd"),
+    ("",      "",      ""),
 ]
diff --git a/build/testdata/069.in b/build/testdata/069.in
index 827c14844..33575a60a 100644
--- a/build/testdata/069.in
+++ b/build/testdata/069.in
@@ -1,2 +1,3 @@
 manifest = [
-    # buildifier: table ("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
+    # buildifier: table
+    ("a", "b", "c"), ("ab", "bc", "cd"), ("", "", ""),]
diff --git a/build/testdata/070.golden b/build/testdata/070.golden
index ccfd39d0b..0312b334b 100644
--- a/build/testdata/070.golden
+++ b/build/testdata/070.golden
@@ -1,6 +1,6 @@
-manifest = [
-   # buildifier: table
-   ("",   "",   ""),
-   ("ab", "bc", "cd"),
-   ("a",  "b",  "c")
+manifest =[
+    # buildifier: table
+    ("",      "",      ""),
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
 ]
diff --git a/build/testdata/070.in b/build/testdata/070.in
index f77994a99..e80895536 100644
--- a/build/testdata/070.in
+++ b/build/testdata/070.in
@@ -1 +1,3 @@
-manifest = [# buildifier: table                   ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
+manifest = [
+    # buildifier: table
+                       ("", "", ""), ("ab", "bc", "cd"),("a", "b", "c")]
diff --git a/build/testdata/071.golden b/build/testdata/071.golden
index 4d302c377..0312b334b 100644
--- a/build/testdata/071.golden
+++ b/build/testdata/071.golden
@@ -1,6 +1,6 @@
-manifest = [
+manifest =[
     # buildifier: table
-    ("",    "",     ""),
-    ("ab",  "bc",   "cd"),
-    ("a",   "b",    "c"),
+    ("",      "",      ""),
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
 ]
diff --git a/build/testdata/072.golden b/build/testdata/072.golden
index c7dee849a..74baf50e8 100644
--- a/build/testdata/072.golden
+++ b/build/testdata/072.golden
@@ -1,3 +1,3 @@
 manifest = [
-# buildifier: table
+    # buildifier: table
 ]
diff --git a/build/testdata/073.golden b/build/testdata/073.golden
index 55f37734f..6804b228d 100644
--- a/build/testdata/073.golden
+++ b/build/testdata/073.golden
@@ -1,6 +1,6 @@
-manifest = [
-   # buildifier: table
-   (),
-   (),
-   ()
+manifest =[
+    # buildifier: table
+    (),
+    (),
+    (),
 ]
diff --git a/build/testdata/074.golden b/build/testdata/074.golden
index ce8c3266b..a927f0dc4 100644
--- a/build/testdata/074.golden
+++ b/build/testdata/074.golden
@@ -1,6 +1,6 @@
-manifest = [
+manifest =[
     # buildifier: table
-    ("",                        "",     ""),
-    ("ab",                      "bc",   "cd"),
-    ("aaaaaaaaaaaaaaaaaaaa",    "b",    "c"),
+    ("",                        "",      ""),
+    ("ab",                      "bc",    "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa",    "b",     "c"),
 ]
diff --git a/build/testdata/075.golden b/build/testdata/075.golden
index 2cd66fa34..eaaec8d3f 100644
--- a/build/testdata/075.golden
+++ b/build/testdata/075.golden
@@ -1,6 +1,6 @@
-manifest = [
+manifest =[
     # buildifier: table
-    ("",                        "",                         ""),
-    ("ab",                      "bcbcbcbcbcbcbcbcbcbcbc",   "cd"),
-    ("aaaaaaaaaaaaaaaaaaaa",    "b",                        "c"),
+    ("",                        "",                          ""),
+    ("ab",                      "bcbcbcbcbcbcbcbcbcbcbc",    "cd"),
+    ("aaaaaaaaaaaaaaaaaaaa",    "b",                         "c"),
 ]

From d087cb52ef3dc107a916ad0e09f09f79f59d5e30 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Thu, 1 Apr 2021 14:45:10 +0530
Subject: [PATCH 18/32] Revise golden files with sorting logic.

---
 build/rewrite.go          | 20 +++++++++-----------
 build/testdata/068.golden |  4 ++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index 5d2a2ce43..0e0ee0e00 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -19,7 +19,6 @@ limitations under the License.
 package build
 
 import (
-	"fmt"
 	"path"
 	"path/filepath"
 	"regexp"
@@ -453,26 +452,25 @@ func formatTables(f *File) {
 					// This is a duplicate of a string above.
 					// Collect comments so that they're not lost.
 					comments = append(comments, tupleRow.Comment().Before...)
-					fmt.Println("RTRT", comments)
 
 					// create a map with <key, tuple> values
 					tableMap[key] = tupleRow
 				}
 
 				// sort the keys
-                                var CommentKey []Comment
-                                for i, key := range keys {
-                                        if i == 0 {
-                                                CommentKey = tableMap[key].Comment().Before[0:1]
-                                                tableMap[key].Comment().Before = tableMap[key].Comment().Before[1:]
-                                        }
-                                }
+				var CommentKey []Comment
+				for i, key := range keys {
+					if i == 0 {
+						CommentKey = tableMap[key].Comment().Before[0:1]
+						tableMap[key].Comment().Before = tableMap[key].Comment().Before[1:]
+					}
+				}
 				sort.Strings(keys)
 				// rewrite the sorted list to v.list
 				for i, key := range keys {
 					if i == 0 {
-	          				tableMap[key].Comment().Before = CommentKey
-                                        }
+						tableMap[key].Comment().Before = CommentKey
+					}
 					sortedList = append(sortedList, tableMap[key])
 				}
 				v.List = sortedList
diff --git a/build/testdata/068.golden b/build/testdata/068.golden
index 582516d37..8a764025b 100644
--- a/build/testdata/068.golden
+++ b/build/testdata/068.golden
@@ -1,6 +1,6 @@
 manifest =[
     # buildifier: table sort 1
-    ("ab",    "bc",    "cd"),
-    ("a",     "b",     "c"),
     ("",      "",      ""),
+    ("a",     "b",     "c"),
+    ("ab",    "bc",    "cd"),
 ]

From f1c491d2a0c0df2784839c37a323102995bbaec0 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Thu, 1 Apr 2021 03:08:37 -0700
Subject: [PATCH 19/32] Test fix for 067 + Code clean up

---
 build/print.go                  | 16 ++++++++--------
 build/rewrite.go                |  4 ++--
 build/syntax.go                 |  4 ++--
 build/testdata/067.build.golden |  4 ++++
 build/testdata/067.bzl.golden   |  1 +
 build/testdata/067.golden       |  1 -
 6 files changed, 17 insertions(+), 13 deletions(-)
 create mode 100644 build/testdata/067.build.golden
 create mode 100644 build/testdata/067.bzl.golden
 delete mode 100644 build/testdata/067.golden

diff --git a/build/print.go b/build/print.go
index 98f468da8..656f590a2 100644
--- a/build/print.go
+++ b/build/print.go
@@ -75,14 +75,14 @@ type printer struct {
 	tw           *tabwriter.Writer // tab writer
 }
 
-// printf prints to the buffer.
+// printf prints to the buffer either directly or via TabWriter based on the mode
 func (p *printer) printf(format string, args ...interface{}) {
 	if !p.tabWriterOn {
 		fmt.Fprintf(p, format, args...)
-	} else {
-		if p.tw != nil {
-			fmt.Fprintf(p.tw, format, args...)
-		}
+		return
+	}
+	if p.tw != nil {
+		fmt.Fprintf(p.tw, format, args...)
 	}
 }
 
@@ -618,7 +618,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false)
 
 	case *ListExpr:
-		if v.ForceTable {
+		if v.ForceTabular {
 			p.tabWriterOn = true
 			p.tw = new(tabwriter.Writer)
 			p.tw.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
@@ -626,7 +626,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 
 		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
 
-		if v.ForceTable {
+		if v.ForceTabular {
 			p.tw.Flush()
 		}
 
@@ -638,7 +638,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		if v.NoBrackets {
 			mode = modeSeq
 		}
-		if v.ForceTableRow {
+		if v.ForceTabular {
 			mode = modeTable
 		}
 		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine)
diff --git a/build/rewrite.go b/build/rewrite.go
index 0e0ee0e00..5911f569b 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -415,14 +415,14 @@ func formatTables(f *File) {
 		case *ListExpr:
 			// Handle when "#buildifier: table" tag is set
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
-				v.ForceTable = true
+				v.ForceTabular = true
 				// Iterate within the items of the list ( tablerows)
 				for _, row := range v.List {
 					tupleRow, ok := row.(*TupleExpr)
 					if !ok {
 						continue
 					}
-					tupleRow.ForceTableRow = true
+					tupleRow.ForceTabular = true
 				}
 			}
 			// Handle when "#buildifier: table sort N" tag is set
diff --git a/build/syntax.go b/build/syntax.go
index 4b5879701..d9d5f8159 100644
--- a/build/syntax.go
+++ b/build/syntax.go
@@ -445,7 +445,7 @@ type ListExpr struct {
 	List  []Expr
 	End
 	ForceMultiLine bool // force multiline form when printing
-	ForceTable     bool // force tablular fornmatting when printing
+	ForceTabular   bool // force tabular formatting when printing
 }
 
 // Span returns the start and end positions of the node
@@ -488,7 +488,7 @@ type TupleExpr struct {
 	End
 	ForceCompact   bool // force compact (non-multiline) form when printing
 	ForceMultiLine bool // force multiline form when printing
-	ForceTableRow  bool // force tablular formatting when printing
+	ForceTabular   bool // force tabular formatting when printing
 }
 
 // Span returns the start and end positions of the node
diff --git a/build/testdata/067.build.golden b/build/testdata/067.build.golden
new file mode 100644
index 000000000..ea1a67800
--- /dev/null
+++ b/build/testdata/067.build.golden
@@ -0,0 +1,4 @@
+manifest = [
+    ("a", "b", "c"),
+    ("ab", "bc", "cd"),
+]
diff --git a/build/testdata/067.bzl.golden b/build/testdata/067.bzl.golden
new file mode 100644
index 000000000..e34f18887
--- /dev/null
+++ b/build/testdata/067.bzl.golden
@@ -0,0 +1 @@
+manifest = [("a", "b", "c"), ("ab", "bc", "cd")]
diff --git a/build/testdata/067.golden b/build/testdata/067.golden
deleted file mode 100644
index b7e1f0bb4..000000000
--- a/build/testdata/067.golden
+++ /dev/null
@@ -1 +0,0 @@
-manifest = [("a", "b", "c"), ("ab", "bc", "cd")]
\ No newline at end of file

From 15e04bb754ff16ea2ef79b74bb8ec9ab560c1fc8 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Thu, 1 Apr 2021 03:27:25 -0700
Subject: [PATCH 20/32] More code clean-up , remove unnecessary variables -
 standardize the table formatting

---
 build/print.go   | 24 ++++++++++--------------
 build/rewrite.go |  4 ++--
 build/syntax.go  |  1 -
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/build/print.go b/build/print.go
index 656f590a2..1510a6ddf 100644
--- a/build/print.go
+++ b/build/print.go
@@ -585,12 +585,12 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.margin = m
 
 	case *ParenExpr:
-		p.seq("()", &v.Start, &[]Expr{v.X}, &v.End, modeParen, false, v.ForceMultiLine)
+		p.seq("()", &v.Start, &[]Expr{v.X}, &v.End, modeParen, false, v.ForceMultiLine, false)
 
 	case *CallExpr:
 		addParen(precSuffix)
 		p.expr(v.X, precSuffix)
-		p.seq("()", &v.ListStart, &v.List, &v.End, modeCall, v.ForceCompact, v.ForceMultiLine)
+		p.seq("()", &v.ListStart, &v.List, &v.End, modeCall, v.ForceCompact, v.ForceMultiLine, false)
 
 	case *LoadStmt:
 		addParen(precSuffix)
@@ -615,7 +615,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 			}
 			args = append(args, arg)
 		}
-		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false)
+		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false, false)
 
 	case *ListExpr:
 		if v.ForceTabular {
@@ -624,31 +624,28 @@ func (p *printer) expr(v Expr, outerPrec int) {
 			p.tw.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
 		}
 
-		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
+		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, p.tabWriterOn)
 
 		if v.ForceTabular {
 			p.tw.Flush()
 		}
 
 	case *SetExpr:
-		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
+		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, false)
 
 	case *TupleExpr:
 		mode := modeTuple
 		if v.NoBrackets {
 			mode = modeSeq
 		}
-		if v.ForceTabular {
-			mode = modeTable
-		}
-		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine)
+		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, p.tabWriterOn)
 
 	case *DictExpr:
 		var list []Expr
 		for _, x := range v.List {
 			list = append(list, x)
 		}
-		p.seq("{}", &v.Start, &list, &v.End, modeDict, false, v.ForceMultiLine)
+		p.seq("{}", &v.Start, &list, &v.End, modeDict, false, v.ForceMultiLine, false)
 
 	case *Comprehension:
 		p.listFor(v)
@@ -671,7 +668,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 	case *DefStmt:
 		p.printf("def ")
 		p.printf(v.Name)
-		p.seq("()", &v.StartPos, &v.Params, nil, modeDef, v.ForceCompact, v.ForceMultiLine)
+		p.seq("()", &v.StartPos, &v.Params, nil, modeDef, v.ForceCompact, v.ForceMultiLine, false)
 		if v.Type != nil {
 			p.printf(" -> ")
 			p.expr(v.Type, precLow)
@@ -771,7 +768,6 @@ const (
 	modeSeq   // x, y
 	modeDef   // def f(x, y)
 	modeLoad  // load(a, b, c)
-	modeTable // [(a,    b,    c)]
 )
 
 // useCompactMode reports whether a sequence should be formatted in a compact mode
@@ -842,7 +838,7 @@ func (p *printer) useCompactMode(start *Position, list *[]Expr, end *End, mode s
 // The mode parameter specifies the sequence mode (see above).
 // If multiLine is true, seq avoids the compact form even
 // for 0- and 1-element sequences.
-func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mode seqMode, forceCompact, forceMultiLine bool) {
+func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mode seqMode, forceCompact, forceMultiLine, forceTabular bool) {
 	if mode != modeSeq {
 		p.printf("%s", brack[:1])
 	}
@@ -854,7 +850,7 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 		}
 	}()
 
-	if mode == modeTable {
+	if mode == modeTuple && forceTabular {
 		for i, x := range *list {
 			if i > 0 {
 				p.printf(",\t")
diff --git a/build/rewrite.go b/build/rewrite.go
index 5911f569b..5285e68ff 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -417,13 +417,13 @@ func formatTables(f *File) {
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
 				v.ForceTabular = true
 				// Iterate within the items of the list ( tablerows)
-				for _, row := range v.List {
+				/*for _, row := range v.List {
 					tupleRow, ok := row.(*TupleExpr)
 					if !ok {
 						continue
 					}
 					tupleRow.ForceTabular = true
-				}
+				}*/
 			}
 			// Handle when "#buildifier: table sort N" tag is set
 			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
diff --git a/build/syntax.go b/build/syntax.go
index d9d5f8159..1a4f7fe15 100644
--- a/build/syntax.go
+++ b/build/syntax.go
@@ -488,7 +488,6 @@ type TupleExpr struct {
 	End
 	ForceCompact   bool // force compact (non-multiline) form when printing
 	ForceMultiLine bool // force multiline form when printing
-	ForceTabular   bool // force tabular formatting when printing
 }
 
 // Span returns the start and end positions of the node

From 45d232664fdbb28970b18897660ab3116edc77a3 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Thu, 1 Apr 2021 16:35:58 +0530
Subject: [PATCH 21/32] Format changes, code cleanup

---
 build/print.go   | 13 +++++++------
 build/rewrite.go | 16 ++++------------
 2 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/build/print.go b/build/print.go
index 1510a6ddf..e56fa2d4e 100644
--- a/build/print.go
+++ b/build/print.go
@@ -72,7 +72,7 @@ type printer struct {
 	level        int               // nesting level of def-, if-else- and for-blocks
 	needsNewLine bool              // true if the next statement needs a new line before it
 	tabWriterOn  bool              // when this mode is on ,use the tabwriter to write to buffer
-	tw           *tabwriter.Writer // tab writer
+	tWriter      *tabwriter.Writer // tab writer
 }
 
 // printf prints to the buffer either directly or via TabWriter based on the mode
@@ -81,8 +81,8 @@ func (p *printer) printf(format string, args ...interface{}) {
 		fmt.Fprintf(p, format, args...)
 		return
 	}
-	if p.tw != nil {
-		fmt.Fprintf(p.tw, format, args...)
+	if p.tWriter != nil {
+		fmt.Fprintf(p.tWriter, format, args...)
 	}
 }
 
@@ -620,14 +620,14 @@ func (p *printer) expr(v Expr, outerPrec int) {
 	case *ListExpr:
 		if v.ForceTabular {
 			p.tabWriterOn = true
-			p.tw = new(tabwriter.Writer)
-			p.tw.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
+			p.tWriter = new(tabwriter.Writer)
+			p.tWriter.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
 		}
 
 		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, p.tabWriterOn)
 
 		if v.ForceTabular {
-			p.tw.Flush()
+			p.tWriter.Flush()
 		}
 
 	case *SetExpr:
@@ -850,6 +850,7 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 		}
 	}()
 
+	// Tuple entries in each column must end with a tab, to table format it.
 	if mode == modeTuple && forceTabular {
 		for i, x := range *list {
 			if i > 0 {
diff --git a/build/rewrite.go b/build/rewrite.go
index 5285e68ff..29d76b5ef 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -416,14 +416,6 @@ func formatTables(f *File) {
 			// Handle when "#buildifier: table" tag is set
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
 				v.ForceTabular = true
-				// Iterate within the items of the list ( tablerows)
-				/*for _, row := range v.List {
-					tupleRow, ok := row.(*TupleExpr)
-					if !ok {
-						continue
-					}
-					tupleRow.ForceTabular = true
-				}*/
 			}
 			// Handle when "#buildifier: table sort N" tag is set
 			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
@@ -432,7 +424,6 @@ func formatTables(f *File) {
 					return
 				}
 
-				var sortedList []Expr
 				var comments []Comment
 				colNumber := tableSort(v.List[0])
 				keys := make([]string, 0, len(v.List))
@@ -458,18 +449,19 @@ func formatTables(f *File) {
 				}
 
 				// sort the keys
-				var CommentKey []Comment
+				var commentKey []Comment
 				for i, key := range keys {
 					if i == 0 {
-						CommentKey = tableMap[key].Comment().Before[0:1]
+						commentKey = tableMap[key].Comment().Before[0:1]
 						tableMap[key].Comment().Before = tableMap[key].Comment().Before[1:]
 					}
 				}
 				sort.Strings(keys)
 				// rewrite the sorted list to v.list
+				var sortedList []Expr
 				for i, key := range keys {
 					if i == 0 {
-						tableMap[key].Comment().Before = CommentKey
+						tableMap[key].Comment().Before = commentKey
 					}
 					sortedList = append(sortedList, tableMap[key])
 				}

From e866e9ccb9aad50e387743d04452aabbeadfae74 Mon Sep 17 00:00:00 2001
From: Hari Preethi S <hhari@vmware.com>
Date: Thu, 1 Apr 2021 16:54:32 +0530
Subject: [PATCH 22/32] Adding README.md for MagicTable

---
 README.md | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 62 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index 0f5be17f1..948b5feb2 100644
--- a/README.md
+++ b/README.md
@@ -1,14 +1,68 @@
-# Buildtools for bazel
+# Bazel
 
-This repository contains developer tools for working with Google's `bazel` buildtool.
+Bazel is an OSS build system developed by Google.
 
-* [buildifier](buildifier/README.md) For formatting BUILD, BUILD.bazel and BUCK files in a standard way
-* [buildozer](buildozer/README.md) For doing command-line operations on these files.
-* [unused_deps](unused_deps/README.md) For finding unneeded dependencies in
-[java_library](https://docs.bazel.build/versions/master/be/java.html#java_library) rules.
+# Buildifier
+
+There exists a couple of developer tools for working with Google's `bazel` buildsystem. Buildifier is one of them for formatting bazel BUILD, BUILD.bazel and BULK files with a standard convention.
 
-[![Build status](https://badge.buildkite.com/6a80fcf7909883296cada2e474286ea627994b9130aed110e2.svg)](https://buildkite.com/bazel/buildtools-postsubmit)
 
 ## Setup
 
-See instructions in each tool's directory.
+* Install Bazel: https://docs.bazel.build/versions/4.0.0/install.html
+* Install Go: https://golang.org/doc/install
+
+### Get the code:
+
+* Download the tar uploaded and untar it. Or,
+* Checkout the repo(https://gitlab.eng.vmware.com/rtabassum/mirrors_github_bazel-buildtools) which is forked from VMware internal gitlab mirrors repo:
+
+	$ git clone git@gitlab.eng.vmware.com:rtabassum/mirrors_github_bazel-buildtools.git
+	$ cd mirrors_github_bazel-buildtools
+
+	Checkout the branch `osborathon-magic-table`. Its pulled from the latest tag `4.0.1`
+	$ git checkout osborathon-magic-table
+
+
+### Build the tool:
+
+	$ bazel build //buildifier
+
+The binary should be built at this location: `bazel-bin/buildifier/buildifier_/buildifier`
+
+
+### Usage:
+
+Use buildifier to create standardized formatting for BUILD and .bazel src files:
+
+    $ bazel-bin/buildifier/buildifier_/buildifier path/to/file
+
+You can also add the built binary to the PATH and directly invoke:
+
+	$ buildifier path/to/file
+
+You can also process multiple files at once:
+
+    $ buildifier path/to/file1 path/to/file2
+
+You can make buildifier automatically find all Starlark files (i.e. BUILD, WORKSPACE, .bzl)
+in a directory recursively:
+
+    $ buildifier -r path/to/dir
+
+To utilize the new tags being added for formating tabular data add tag in your src file above the tabular data.
+`# buildifier: table`.
+
+Buildifier will properly format the tabular data when you run the Buildifier on your src file.
+
+	$ bazel-bin/buildifier/buildifier_/buildifier path/to/file
+
+Also, to sort the table in your source file by a specific column, use tag "# buildifier: table sort <column_index>"
+
+
+
+## Run unittests
+
+You can run all the default unittests:
+
+$ bazel test --test_output=all //build:go_default_test
\ No newline at end of file

From 495a548ce69be7304a44d8ec677a98a84b3ab092 Mon Sep 17 00:00:00 2001
From: Shwetha Kashyap <kshwetha@vmware.com>
Date: Thu, 1 Apr 2021 04:55:39 -0700
Subject: [PATCH 23/32] Additional documentation of usage in Readme

---
 README.md | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index 948b5feb2..94eff9e31 100644
--- a/README.md
+++ b/README.md
@@ -31,7 +31,7 @@ There exists a couple of developer tools for working with Google's `bazel` build
 The binary should be built at this location: `bazel-bin/buildifier/buildifier_/buildifier`
 
 
-### Usage:
+### General Usage:
 
 Use buildifier to create standardized formatting for BUILD and .bazel src files:
 
@@ -50,12 +50,21 @@ in a directory recursively:
 
     $ buildifier -r path/to/dir
 
+### Using table formating ( Magic table ):
 To utilize the new tags being added for formating tabular data add tag in your src file above the tabular data.
 `# buildifier: table`.
 
+	$ cat sample.bzl
+	manifest =[
+      #buildifier: table
+      ("aaaaaa",     "b",     "c"),
+      ("ab",    "bc",    "cdddd"),
+      ("ab",    "bccccc",    "cd"),
+    ]
+
 Buildifier will properly format the tabular data when you run the Buildifier on your src file.
 
-	$ bazel-bin/buildifier/buildifier_/buildifier path/to/file
+	$ bazel-bin/buildifier/buildifier_/buildifier --lint=fix path/to/sample.bzl
 
 Also, to sort the table in your source file by a specific column, use tag "# buildifier: table sort <column_index>"
 
@@ -65,4 +74,4 @@ Also, to sort the table in your source file by a specific column, use tag "# bui
 
 You can run all the default unittests:
 
-$ bazel test --test_output=all //build:go_default_test
\ No newline at end of file
+$ bazel test --test_output=all //build:go_default_test

From 9727036a1b08b99ef78cbf88159aa3cd6e46196b Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Thu, 17 Jun 2021 17:36:39 -0700
Subject: [PATCH 24/32] Cleanup README.md

---
 README.md | 79 ++++++-------------------------------------------------
 1 file changed, 8 insertions(+), 71 deletions(-)

diff --git a/README.md b/README.md
index 94eff9e31..0f5be17f1 100644
--- a/README.md
+++ b/README.md
@@ -1,77 +1,14 @@
-# Bazel
+# Buildtools for bazel
 
-Bazel is an OSS build system developed by Google.
+This repository contains developer tools for working with Google's `bazel` buildtool.
 
-# Buildifier
-
-There exists a couple of developer tools for working with Google's `bazel` buildsystem. Buildifier is one of them for formatting bazel BUILD, BUILD.bazel and BULK files with a standard convention.
+* [buildifier](buildifier/README.md) For formatting BUILD, BUILD.bazel and BUCK files in a standard way
+* [buildozer](buildozer/README.md) For doing command-line operations on these files.
+* [unused_deps](unused_deps/README.md) For finding unneeded dependencies in
+[java_library](https://docs.bazel.build/versions/master/be/java.html#java_library) rules.
 
+[![Build status](https://badge.buildkite.com/6a80fcf7909883296cada2e474286ea627994b9130aed110e2.svg)](https://buildkite.com/bazel/buildtools-postsubmit)
 
 ## Setup
 
-* Install Bazel: https://docs.bazel.build/versions/4.0.0/install.html
-* Install Go: https://golang.org/doc/install
-
-### Get the code:
-
-* Download the tar uploaded and untar it. Or,
-* Checkout the repo(https://gitlab.eng.vmware.com/rtabassum/mirrors_github_bazel-buildtools) which is forked from VMware internal gitlab mirrors repo:
-
-	$ git clone git@gitlab.eng.vmware.com:rtabassum/mirrors_github_bazel-buildtools.git
-	$ cd mirrors_github_bazel-buildtools
-
-	Checkout the branch `osborathon-magic-table`. Its pulled from the latest tag `4.0.1`
-	$ git checkout osborathon-magic-table
-
-
-### Build the tool:
-
-	$ bazel build //buildifier
-
-The binary should be built at this location: `bazel-bin/buildifier/buildifier_/buildifier`
-
-
-### General Usage:
-
-Use buildifier to create standardized formatting for BUILD and .bazel src files:
-
-    $ bazel-bin/buildifier/buildifier_/buildifier path/to/file
-
-You can also add the built binary to the PATH and directly invoke:
-
-	$ buildifier path/to/file
-
-You can also process multiple files at once:
-
-    $ buildifier path/to/file1 path/to/file2
-
-You can make buildifier automatically find all Starlark files (i.e. BUILD, WORKSPACE, .bzl)
-in a directory recursively:
-
-    $ buildifier -r path/to/dir
-
-### Using table formating ( Magic table ):
-To utilize the new tags being added for formating tabular data add tag in your src file above the tabular data.
-`# buildifier: table`.
-
-	$ cat sample.bzl
-	manifest =[
-      #buildifier: table
-      ("aaaaaa",     "b",     "c"),
-      ("ab",    "bc",    "cdddd"),
-      ("ab",    "bccccc",    "cd"),
-    ]
-
-Buildifier will properly format the tabular data when you run the Buildifier on your src file.
-
-	$ bazel-bin/buildifier/buildifier_/buildifier --lint=fix path/to/sample.bzl
-
-Also, to sort the table in your source file by a specific column, use tag "# buildifier: table sort <column_index>"
-
-
-
-## Run unittests
-
-You can run all the default unittests:
-
-$ bazel test --test_output=all //build:go_default_test
+See instructions in each tool's directory.

From 376034e85e5c0588b7d523ce70698b3408a34e39 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Sat, 19 Jun 2021 14:40:33 -0700
Subject: [PATCH 25/32] Add unit tests for buildifier sorting tag

76 - long tuple entry when sorting
77 - sort tag added on empty table
78 - sort tag added when table in one line
---
 build/testdata/076.golden | 7 +++++++
 build/testdata/076.in     | 7 +++++++
 build/testdata/077.golden | 3 +++
 build/testdata/077.in     | 4 ++++
 build/testdata/078.golden | 6 ++++++
 build/testdata/078.in     | 4 ++++
 6 files changed, 31 insertions(+)
 create mode 100644 build/testdata/076.golden
 create mode 100644 build/testdata/076.in
 create mode 100644 build/testdata/077.golden
 create mode 100644 build/testdata/077.in
 create mode 100644 build/testdata/078.golden
 create mode 100644 build/testdata/078.in

diff --git a/build/testdata/076.golden b/build/testdata/076.golden
new file mode 100644
index 000000000..e40132df3
--- /dev/null
+++ b/build/testdata/076.golden
@@ -0,0 +1,7 @@
+manifest =[
+    # buildifier: table sort 2
+    ("",      "",                   ""),
+    ("a",     "b",                  "c"),
+    ("ab",    "bc",                 "cd"),
+    ("ss",    "sssdsssssssdsss",    "a"),
+]
diff --git a/build/testdata/076.in b/build/testdata/076.in
new file mode 100644
index 000000000..cee043002
--- /dev/null
+++ b/build/testdata/076.in
@@ -0,0 +1,7 @@
+manifest = [
+   # buildifier: table sort 2
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c"),
+   ("",   "",    ""),
+   ("ss", "sssdsssssssdsss", "a"),
+]
\ No newline at end of file
diff --git a/build/testdata/077.golden b/build/testdata/077.golden
new file mode 100644
index 000000000..61a1f3edf
--- /dev/null
+++ b/build/testdata/077.golden
@@ -0,0 +1,3 @@
+manifest = [
+    # buildifier: table sort 2
+]
diff --git a/build/testdata/077.in b/build/testdata/077.in
new file mode 100644
index 000000000..0f8ea87b7
--- /dev/null
+++ b/build/testdata/077.in
@@ -0,0 +1,4 @@
+manifest = [
+   # buildifier: table sort 2
+
+]
\ No newline at end of file
diff --git a/build/testdata/078.golden b/build/testdata/078.golden
new file mode 100644
index 000000000..423deb58a
--- /dev/null
+++ b/build/testdata/078.golden
@@ -0,0 +1,6 @@
+manifest =[
+    # buildifier: table sort 2
+    ("",      "",      ""),
+    ("a",     "b",     "c"),
+    ("ab",    "bc",    "cd"),
+]
diff --git a/build/testdata/078.in b/build/testdata/078.in
new file mode 100644
index 000000000..3eb8a1185
--- /dev/null
+++ b/build/testdata/078.in
@@ -0,0 +1,4 @@
+manifest = [
+   # buildifier: table sort 2
+   ("", "", ""), ("ab", "bc", "cd"), ("a", "b", "c")
+]
\ No newline at end of file

From 636d6bd4afa47d8653b89f7f799ac6e75aee93ad Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Sat, 19 Jun 2021 14:47:41 -0700
Subject: [PATCH 26/32] Bug Fix - Invalid column number specified in sorting
 tag.

Handle buildifier failure with Panic error when the column number
specified in table sort tag is larger than maximun number of columns
or out of range. Also added unit tests.
---
 build/rewrite.go          | 8 +++++++-
 build/testdata/079.golden | 6 ++++++
 build/testdata/079.in     | 6 ++++++
 build/testdata/080.golden | 6 ++++++
 build/testdata/080.in     | 6 ++++++
 5 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 build/testdata/079.golden
 create mode 100644 build/testdata/079.in
 create mode 100644 build/testdata/080.golden
 create mode 100644 build/testdata/080.in

diff --git a/build/rewrite.go b/build/rewrite.go
index 29d76b5ef..2867b69c1 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -428,6 +428,13 @@ func formatTables(f *File) {
 				colNumber := tableSort(v.List[0])
 				keys := make([]string, 0, len(v.List))
 				tableMap := make(map[string]*TupleExpr)
+
+				// column number specified in tag is larger than the max number of columns
+				columns, ok := v.List[1].(*TupleExpr)
+				if !ok || colNumber > len(columns.List) {
+					return
+				}
+
 				// Iterate  over the list of tuples(tablerows)
 				for _, row := range v.List {
 					tupleRow, ok := row.(*TupleExpr)
@@ -440,7 +447,6 @@ func formatTables(f *File) {
 					key := str.Value
 					keys = append(keys, key)
 
-					// This is a duplicate of a string above.
 					// Collect comments so that they're not lost.
 					comments = append(comments, tupleRow.Comment().Before...)
 
diff --git a/build/testdata/079.golden b/build/testdata/079.golden
new file mode 100644
index 000000000..a5a485a12
--- /dev/null
+++ b/build/testdata/079.golden
@@ -0,0 +1,6 @@
+manifest =[
+    # buildifier: table sort 7
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
+    ("",      "",      ""),
+]
diff --git a/build/testdata/079.in b/build/testdata/079.in
new file mode 100644
index 000000000..aa901f113
--- /dev/null
+++ b/build/testdata/079.in
@@ -0,0 +1,6 @@
+manifest = [
+   # buildifier: table sort 7
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c"),
+   ("",   "",    ""),
+]
\ No newline at end of file
diff --git a/build/testdata/080.golden b/build/testdata/080.golden
new file mode 100644
index 000000000..8231f8610
--- /dev/null
+++ b/build/testdata/080.golden
@@ -0,0 +1,6 @@
+manifest =[
+    # buildifier: table sort -2
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
+    ("",      "",      ""),
+]
diff --git a/build/testdata/080.in b/build/testdata/080.in
new file mode 100644
index 000000000..91eba797c
--- /dev/null
+++ b/build/testdata/080.in
@@ -0,0 +1,6 @@
+manifest = [
+   # buildifier: table sort -2
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c"),
+   ("",   "",    ""),
+]
\ No newline at end of file

From 8b787fb187d18a1e634f95021cac34466d9049f8 Mon Sep 17 00:00:00 2001
From: Rehana Tabassum <rtabassum@vmware.com>
Date: Sat, 19 Jun 2021 19:00:34 -0700
Subject: [PATCH 27/32] Handle case when duplicate keys for the table column to
 be sorted.

When buildifier table sort for a column that has duplicate values,
then buildifier sorts the entries preserving the order of the duplicate keys
from the original table. Also added unitetests.

Unit testcase 81 - duplicate key for the col to be sorted for table sort tag, use case when comment appends multiple times in output
Unit testcase 82 - duplicate key for the col to be sorted for table sort tag - throws error
---
 build/rewrite.go          | 26 +++++++++++++++++---------
 build/testdata/076.in     |  2 +-
 build/testdata/077.in     |  2 +-
 build/testdata/078.in     |  2 +-
 build/testdata/079.in     |  2 +-
 build/testdata/080.in     |  2 +-
 build/testdata/081.golden |  6 ++++++
 build/testdata/081.in     |  6 ++++++
 build/testdata/082.golden |  6 ++++++
 build/testdata/082.in     |  6 ++++++
 10 files changed, 46 insertions(+), 14 deletions(-)
 create mode 100644 build/testdata/081.golden
 create mode 100644 build/testdata/081.in
 create mode 100644 build/testdata/082.golden
 create mode 100644 build/testdata/082.in

diff --git a/build/rewrite.go b/build/rewrite.go
index 2867b69c1..87bfc9087 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -427,7 +427,7 @@ func formatTables(f *File) {
 				var comments []Comment
 				colNumber := tableSort(v.List[0])
 				keys := make([]string, 0, len(v.List))
-				tableMap := make(map[string]*TupleExpr)
+				tableMap := map[string][]*TupleExpr{}
 
 				// column number specified in tag is larger than the max number of columns
 				columns, ok := v.List[1].(*TupleExpr)
@@ -450,26 +450,34 @@ func formatTables(f *File) {
 					// Collect comments so that they're not lost.
 					comments = append(comments, tupleRow.Comment().Before...)
 
-					// create a map with <key, tuple> values
-					tableMap[key] = tupleRow
+					// Create a map with <key, tuple> values
+					tableMap[key] = append(tableMap[key], tupleRow)
 				}
 
-				// sort the keys
+				// Store the comment so its not lost
 				var commentKey []Comment
 				for i, key := range keys {
 					if i == 0 {
-						commentKey = tableMap[key].Comment().Before[0:1]
-						tableMap[key].Comment().Before = tableMap[key].Comment().Before[1:]
+						commentKey = tableMap[key][0].Comment().Before[0:1]
+						tableMap[key][0].Comment().Before = tableMap[key][0].Comment().Before[1:]
 					}
 				}
+				// Sort the keys
 				sort.Strings(keys)
-				// rewrite the sorted list to v.list
+
+				// Rewrite the sorted list to v.list
 				var sortedList []Expr
+				alreadySeen := make(map[string]bool)
 				for i, key := range keys {
 					if i == 0 {
-						tableMap[key].Comment().Before = commentKey
+						tableMap[key][0].Comment().Before = commentKey
+					}
+					if _, ok := alreadySeen[key]; !ok {
+						for _, value := range tableMap[key] {
+							sortedList = append(sortedList, value)
+						}
+						alreadySeen[key] = true
 					}
-					sortedList = append(sortedList, tableMap[key])
 				}
 				v.List = sortedList
 			}
diff --git a/build/testdata/076.in b/build/testdata/076.in
index cee043002..166af2452 100644
--- a/build/testdata/076.in
+++ b/build/testdata/076.in
@@ -4,4 +4,4 @@ manifest = [
    ("a",  "b",  "c"),
    ("",   "",    ""),
    ("ss", "sssdsssssssdsss", "a"),
-]
\ No newline at end of file
+]
diff --git a/build/testdata/077.in b/build/testdata/077.in
index 0f8ea87b7..9b4e98a05 100644
--- a/build/testdata/077.in
+++ b/build/testdata/077.in
@@ -1,4 +1,4 @@
 manifest = [
    # buildifier: table sort 2
 
-]
\ No newline at end of file
+]
diff --git a/build/testdata/078.in b/build/testdata/078.in
index 3eb8a1185..900a8bad7 100644
--- a/build/testdata/078.in
+++ b/build/testdata/078.in
@@ -1,4 +1,4 @@
 manifest = [
    # buildifier: table sort 2
    ("", "", ""), ("ab", "bc", "cd"), ("a", "b", "c")
-]
\ No newline at end of file
+]
diff --git a/build/testdata/079.in b/build/testdata/079.in
index aa901f113..3e0a4891b 100644
--- a/build/testdata/079.in
+++ b/build/testdata/079.in
@@ -3,4 +3,4 @@ manifest = [
    ("ab", "bc", "cd"),
    ("a",  "b",  "c"),
    ("",   "",    ""),
-]
\ No newline at end of file
+]
diff --git a/build/testdata/080.in b/build/testdata/080.in
index 91eba797c..42f34cbf5 100644
--- a/build/testdata/080.in
+++ b/build/testdata/080.in
@@ -3,4 +3,4 @@ manifest = [
    ("ab", "bc", "cd"),
    ("a",  "b",  "c"),
    ("",   "",    ""),
-]
\ No newline at end of file
+]
diff --git a/build/testdata/081.golden b/build/testdata/081.golden
new file mode 100644
index 000000000..7b14928e7
--- /dev/null
+++ b/build/testdata/081.golden
@@ -0,0 +1,6 @@
+manifest =[
+    # buildifier: table sort 1
+    ("a",     "bd",    "ff"),
+    ("a",     "b",     "cc"),
+    ("ab",    "bc",    "cd"),
+]
diff --git a/build/testdata/081.in b/build/testdata/081.in
new file mode 100644
index 000000000..35c3da14a
--- /dev/null
+++ b/build/testdata/081.in
@@ -0,0 +1,6 @@
+manifest = [
+   # buildifier: table sort 1
+   ("ab", "bc", "cd"),
+   ("a",  "bd",  "ff"),
+   ("a",  "b",  "cc"),
+]
diff --git a/build/testdata/082.golden b/build/testdata/082.golden
new file mode 100644
index 000000000..851cad499
--- /dev/null
+++ b/build/testdata/082.golden
@@ -0,0 +1,6 @@
+manifest =[
+    # buildifier: table sort 1
+    ("a",     "b",     "cc"),
+    ("ab",    "bc",    "cd"),
+    ("ab",    "b",     "ff"),
+]
diff --git a/build/testdata/082.in b/build/testdata/082.in
new file mode 100644
index 000000000..ad489c47d
--- /dev/null
+++ b/build/testdata/082.in
@@ -0,0 +1,6 @@
+manifest = [
+   # buildifier: table sort 1
+   ("ab", "bc", "cd"),
+   ("ab",  "b",  "ff"),
+   ("a",  "b",  "cc"),
+]

From 563207f1af9b87f3df5cf38240bb2f68e87d5693 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Sun, 20 Jun 2021 22:09:53 -0700
Subject: [PATCH 28/32] Fix the "missing space" issue with some design changes

Design changes made here :
1) Seperate the out the logic required to print tabular data into a new secluded method.
2) Avoid "mid-line" changes of `io.writer` to avoid removal of spaces within statements.

Also make accompanying test changes.
---
 build/print.go            | 74 +++++++++++++++++++++++++--------------
 build/testdata/068.golden |  2 +-
 build/testdata/069.golden |  2 +-
 build/testdata/070.golden |  2 +-
 build/testdata/071.golden |  2 +-
 build/testdata/073.golden |  2 +-
 build/testdata/074.golden |  2 +-
 build/testdata/075.golden |  2 +-
 build/testdata/076.golden |  2 +-
 build/testdata/078.golden |  2 +-
 build/testdata/079.golden |  2 +-
 build/testdata/080.golden |  2 +-
 build/testdata/081.golden |  2 +-
 build/testdata/082.golden |  2 +-
 14 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/build/print.go b/build/print.go
index e56fa2d4e..491a8fb04 100644
--- a/build/print.go
+++ b/build/print.go
@@ -618,17 +618,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false, false)
 
 	case *ListExpr:
-		if v.ForceTabular {
-			p.tabWriterOn = true
-			p.tWriter = new(tabwriter.Writer)
-			p.tWriter.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
-		}
-
-		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, p.tabWriterOn)
-
-		if v.ForceTabular {
-			p.tWriter.Flush()
-		}
+		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, v.ForceTabular)
 
 	case *SetExpr:
 		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, false)
@@ -638,7 +628,12 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		if v.NoBrackets {
 			mode = modeSeq
 		}
-		p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, p.tabWriterOn)
+
+		if !p.tabWriterOn {
+			p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, false)
+		} else {
+			p.tabbedSeq("()", &v.Start, &v.List, &v.End)
+		}
 
 	case *DictExpr:
 		var list []Expr
@@ -842,7 +837,19 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 	if mode != modeSeq {
 		p.printf("%s", brack[:1])
 	}
+
 	p.depth++
+	if forceTabular {
+		p.tabWriterOn = true
+		p.tWriter = new(tabwriter.Writer)
+		p.tWriter.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
+	}
+	defer func() {
+		if forceTabular {
+			p.tWriter.Flush()
+		}
+	}()
+
 	defer func() {
 		p.depth--
 		if mode != modeSeq {
@@ -850,21 +857,6 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 		}
 	}()
 
-	// Tuple entries in each column must end with a tab, to table format it.
-	if mode == modeTuple && forceTabular {
-		for i, x := range *list {
-			if i > 0 {
-				p.printf(",\t")
-			}
-			p.expr(x, precLow)
-		}
-		// Single-element tuple must end with comma, to mark it as a tuple.
-		if len(*list) == 1 && mode == modeTuple {
-			p.printf(",")
-		}
-		return
-	}
-
 	if p.useCompactMode(start, list, end, mode, forceCompact, forceMultiLine) {
 		for i, x := range *list {
 			if i > 0 {
@@ -918,6 +910,34 @@ func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mod
 	}
 }
 
+// Formats a list of values inside a given bracket pair (brack = "()", "[]")
+// by adding a tabspace in between, so that the tabwriter prints them out in table format.
+func (p *printer) tabbedSeq(brack string, start *Position, list *[]Expr, end *End) {
+
+	// Print starting braces
+	p.printf("%s", brack[:1])
+
+	p.depth++
+	defer func() {
+		p.depth--
+		p.printf("%s", brack[1:])
+
+	}()
+
+	// Tablerows entries in each column must end with a tab, to table format it.
+	for i, x := range *list {
+		if i > 0 {
+			p.printf(",\t")
+		}
+		p.expr(x, precLow)
+	}
+
+	// Single-element tuple must end with comma, to mark it as a tuple.
+	if len(*list) == 1 {
+		p.printf(",")
+	}
+}
+
 func needsTrailingComma(mode seqMode, v Expr) bool {
 	switch mode {
 	case modeDef:
diff --git a/build/testdata/068.golden b/build/testdata/068.golden
index 8a764025b..22187af6d 100644
--- a/build/testdata/068.golden
+++ b/build/testdata/068.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 1
     ("",      "",      ""),
     ("a",     "b",     "c"),
diff --git a/build/testdata/069.golden b/build/testdata/069.golden
index 05799bdf7..8188de7ae 100644
--- a/build/testdata/069.golden
+++ b/build/testdata/069.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     ("a",     "b",     "c"),
     ("ab",    "bc",    "cd"),
diff --git a/build/testdata/070.golden b/build/testdata/070.golden
index 0312b334b..fde25e060 100644
--- a/build/testdata/070.golden
+++ b/build/testdata/070.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     ("",      "",      ""),
     ("ab",    "bc",    "cd"),
diff --git a/build/testdata/071.golden b/build/testdata/071.golden
index 0312b334b..fde25e060 100644
--- a/build/testdata/071.golden
+++ b/build/testdata/071.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     ("",      "",      ""),
     ("ab",    "bc",    "cd"),
diff --git a/build/testdata/073.golden b/build/testdata/073.golden
index 6804b228d..c826230d1 100644
--- a/build/testdata/073.golden
+++ b/build/testdata/073.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     (),
     (),
diff --git a/build/testdata/074.golden b/build/testdata/074.golden
index a927f0dc4..325939b2d 100644
--- a/build/testdata/074.golden
+++ b/build/testdata/074.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     ("",                        "",      ""),
     ("ab",                      "bc",    "cd"),
diff --git a/build/testdata/075.golden b/build/testdata/075.golden
index eaaec8d3f..b6cef31e3 100644
--- a/build/testdata/075.golden
+++ b/build/testdata/075.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table
     ("",                        "",                          ""),
     ("ab",                      "bcbcbcbcbcbcbcbcbcbcbc",    "cd"),
diff --git a/build/testdata/076.golden b/build/testdata/076.golden
index e40132df3..51932f76c 100644
--- a/build/testdata/076.golden
+++ b/build/testdata/076.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 2
     ("",      "",                   ""),
     ("a",     "b",                  "c"),
diff --git a/build/testdata/078.golden b/build/testdata/078.golden
index 423deb58a..fa967a39d 100644
--- a/build/testdata/078.golden
+++ b/build/testdata/078.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 2
     ("",      "",      ""),
     ("a",     "b",     "c"),
diff --git a/build/testdata/079.golden b/build/testdata/079.golden
index a5a485a12..e2cad6c38 100644
--- a/build/testdata/079.golden
+++ b/build/testdata/079.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 7
     ("ab",    "bc",    "cd"),
     ("a",     "b",     "c"),
diff --git a/build/testdata/080.golden b/build/testdata/080.golden
index 8231f8610..b8879fc76 100644
--- a/build/testdata/080.golden
+++ b/build/testdata/080.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort -2
     ("ab",    "bc",    "cd"),
     ("a",     "b",     "c"),
diff --git a/build/testdata/081.golden b/build/testdata/081.golden
index 7b14928e7..4e3e95f1c 100644
--- a/build/testdata/081.golden
+++ b/build/testdata/081.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 1
     ("a",     "bd",    "ff"),
     ("a",     "b",     "cc"),
diff --git a/build/testdata/082.golden b/build/testdata/082.golden
index 851cad499..9ea4e96f2 100644
--- a/build/testdata/082.golden
+++ b/build/testdata/082.golden
@@ -1,4 +1,4 @@
-manifest =[
+manifest = [
     # buildifier: table sort 1
     ("a",     "b",     "cc"),
     ("ab",    "bc",    "cd"),

From 3ceb9178db66d50874b00ce57f1a168030d29b5b Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Mon, 21 Jun 2021 13:31:34 -0700
Subject: [PATCH 29/32] Refactoring `formatTables` function

1) Seperate the "Tag detection" logic from the "sorting logic"
2) `formatTables` onlys Walks the AST and marks nodes as `contender for formatting as table`
3) `sortTableRows` takes care of sorting
---
 build/rewrite.go | 127 +++++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 60 deletions(-)

diff --git a/build/rewrite.go b/build/rewrite.go
index 87bfc9087..3cddc13da 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -406,83 +406,90 @@ func (x namedArgs) Less(i, j int) bool {
 }
 
 // Detects if a data is marked as tablular based on the tag `buildifier: table`
-// and sets the correct flags on the expression.
-// If the tags necessitate a sorting of the tabular data, then this takes care of inplace sorting.
+// and sets the correct flags on the expression to indentify those nodes in the AST.
 func formatTables(f *File) {
-	Walk(f, func(v Expr, stk []Expr) {
+	markTableNodes := func(v Expr, stk []Expr) {
 		switch v := v.(type) {
 		// Tabular formatting is currently supported only for lists
 		case *ListExpr:
 			// Handle when "#buildifier: table" tag is set
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
 				v.ForceTabular = true
+				sortTableRows(v)
 			}
-			// Handle when "#buildifier: table sort N" tag is set
-			if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
-				// less than 2 rows, nothing to sort
-				if len(v.List) < 2 {
-					return
-				}
+		}
+	}
 
-				var comments []Comment
-				colNumber := tableSort(v.List[0])
-				keys := make([]string, 0, len(v.List))
-				tableMap := map[string][]*TupleExpr{}
+	Walk(f, markTableNodes)
+}
 
-				// column number specified in tag is larger than the max number of columns
-				columns, ok := v.List[1].(*TupleExpr)
-				if !ok || colNumber > len(columns.List) {
-					return
-				}
+// If the tags necessitate a sorting of the tabular data, then this takes care of sorting.
+// TODO : Change to in-place sorting later.
+func sortTableRows(v *ListExpr) {
+	// Handle when "#buildifier: table sort N" tag is set
+	if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
+		// less than 2 rows, nothing to sort
+		if len(v.List) < 2 {
+			return
+		}
 
-				// Iterate  over the list of tuples(tablerows)
-				for _, row := range v.List {
-					tupleRow, ok := row.(*TupleExpr)
-					if !ok {
-						continue
-					}
-					// keys are the values from tuple[colNumber position]
-					// Table column starts at 1, where list starts at 0
-					str, ok := tupleRow.List[colNumber-1].(*StringExpr)
-					key := str.Value
-					keys = append(keys, key)
+		var comments []Comment
+		colNumber := tableSort(v.List[0])
+		keys := make([]string, 0, len(v.List))
+		tableMap := map[string][]*TupleExpr{}
 
-					// Collect comments so that they're not lost.
-					comments = append(comments, tupleRow.Comment().Before...)
+		// column number specified in tag is larger than the max number of columns
+		columns, ok := v.List[1].(*TupleExpr)
+		if !ok || colNumber > len(columns.List) {
+			return
+		}
 
-					// Create a map with <key, tuple> values
-					tableMap[key] = append(tableMap[key], tupleRow)
-				}
+		// Iterate  over the list of tuples(tablerows)
+		for _, row := range v.List {
+			tupleRow, ok := row.(*TupleExpr)
+			if !ok {
+				continue
+			}
+			// keys are the values from tuple[colNumber position]
+			// Table column starts at 1, where list starts at 0
+			str, ok := tupleRow.List[colNumber-1].(*StringExpr)
+			key := str.Value
+			keys = append(keys, key)
 
-				// Store the comment so its not lost
-				var commentKey []Comment
-				for i, key := range keys {
-					if i == 0 {
-						commentKey = tableMap[key][0].Comment().Before[0:1]
-						tableMap[key][0].Comment().Before = tableMap[key][0].Comment().Before[1:]
-					}
-				}
-				// Sort the keys
-				sort.Strings(keys)
-
-				// Rewrite the sorted list to v.list
-				var sortedList []Expr
-				alreadySeen := make(map[string]bool)
-				for i, key := range keys {
-					if i == 0 {
-						tableMap[key][0].Comment().Before = commentKey
-					}
-					if _, ok := alreadySeen[key]; !ok {
-						for _, value := range tableMap[key] {
-							sortedList = append(sortedList, value)
-						}
-						alreadySeen[key] = true
-					}
+			// Collect comments so that they're not lost.
+			comments = append(comments, tupleRow.Comment().Before...)
+
+			// Create a map with <key, tuple> values
+			tableMap[key] = append(tableMap[key], tupleRow)
+		}
+
+		// Store the comment so its not lost
+		var commentKey []Comment
+		for i, key := range keys {
+			if i == 0 {
+				commentKey = tableMap[key][0].Comment().Before[0:1]
+				tableMap[key][0].Comment().Before = tableMap[key][0].Comment().Before[1:]
+			}
+		}
+		// Sort the keys
+		sort.Strings(keys)
+
+		// Rewrite the sorted list to v.list
+		var sortedList []Expr
+		alreadySeen := make(map[string]bool)
+		for i, key := range keys {
+			if i == 0 {
+				tableMap[key][0].Comment().Before = commentKey
+			}
+			if _, ok := alreadySeen[key]; !ok {
+				for _, value := range tableMap[key] {
+					sortedList = append(sortedList, value)
 				}
-				v.List = sortedList
+				alreadySeen[key] = true
 			}
 		}
-	})
+		v.List = sortedList
+	}
 }
 
 // sortStringLists sorts lists of string literals used as specific rule arguments.

From dc76563c92bdef8ad8add417e3594f1e0b3c3a66 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Tue, 22 Jun 2021 01:01:54 -0700
Subject: [PATCH 30/32] Completion of Design 1: - Till now only the Table's
 root node was detected and marked by our logic. - Rows were printed based on
 whether "tabWriter" is ON/OFF. - This makes the logic brittle. - Add code to
 detect the children of Table root which act as the rows of the table. -
 Marking them gives us a flexible design - that can be extended to [ [ ] ]/(
 [] ) etc

---
 build/print.go   |  8 ++++----
 build/rewrite.go | 18 +++++++++++++++---
 build/syntax.go  |  7 ++++---
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/build/print.go b/build/print.go
index 491a8fb04..bcf5abc16 100644
--- a/build/print.go
+++ b/build/print.go
@@ -618,7 +618,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false, false)
 
 	case *ListExpr:
-		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, v.ForceTabular)
+		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, v.FormatAsTable)
 
 	case *SetExpr:
 		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, false)
@@ -629,10 +629,10 @@ func (p *printer) expr(v Expr, outerPrec int) {
 			mode = modeSeq
 		}
 
-		if !p.tabWriterOn {
-			p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, false)
-		} else {
+		if v.FormatAsTableRow {
 			p.tabbedSeq("()", &v.Start, &v.List, &v.End)
+		} else {
+			p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, false)
 		}
 
 	case *DictExpr:
diff --git a/build/rewrite.go b/build/rewrite.go
index 3cddc13da..4fcbe27b9 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -405,16 +405,28 @@ func (x namedArgs) Less(i, j int) bool {
 	return p.index < q.index
 }
 
-// Detects if a data is marked as tablular based on the tag `buildifier: table`
-// and sets the correct flags on the expression to indentify those nodes in the AST.
+// Detects if a data is marked as tablular based on the tag
+// `buildifier: table` and sets the correct flags on the expression to
+// identify those nodes in the AST.
 func formatTables(f *File) {
+
+	// Function that sets a bit on the child nodes of type (list) to indicate
+	// that they are a part of a row
+	markTableRowNodes := func(x Expr, stk2 []Expr) Expr {
+		if x, ok := x.(*TupleExpr); ok {
+			x.FormatAsTableRow = true
+		}
+		return x
+	}
+
 	markTableNodes := func(v Expr, stk []Expr) {
 		switch v := v.(type) {
 		// Tabular formatting is currently supported only for lists
 		case *ListExpr:
 			// Handle when "#buildifier: table" tag is set
 			if len(v.List) > 0 && tableFormat(v.List[0]) {
-				v.ForceTabular = true
+				v.FormatAsTable = true
+				EditChildren(v, markTableRowNodes)
 				sortTableRows(v)
 			}
 		}
diff --git a/build/syntax.go b/build/syntax.go
index 1a4f7fe15..27a53d3c3 100644
--- a/build/syntax.go
+++ b/build/syntax.go
@@ -445,7 +445,7 @@ type ListExpr struct {
 	List  []Expr
 	End
 	ForceMultiLine bool // force multiline form when printing
-	ForceTabular   bool // force tabular formatting when printing
+	FormatAsTable  bool // flag to indicate that this list needs to print tabular data.
 }
 
 // Span returns the start and end positions of the node
@@ -486,8 +486,9 @@ type TupleExpr struct {
 	Start      Position
 	List       []Expr
 	End
-	ForceCompact   bool // force compact (non-multiline) form when printing
-	ForceMultiLine bool // force multiline form when printing
+	ForceCompact     bool // force compact (non-multiline) form when printing
+	ForceMultiLine   bool // force multiline form when printing
+	FormatAsTableRow bool // flag that indicates that this tuple will be printed as a row of a table
 }
 
 // Span returns the start and end positions of the node

From 9ba01a4c615c547e59d88da8be1ab823fd2dfde8 Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Tue, 22 Jun 2021 01:37:52 -0700
Subject: [PATCH 31/32] Code cleanup- Add back mode `modeTable` instead of
 extra param in `seq` call

---
 build/print.go | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/build/print.go b/build/print.go
index bcf5abc16..2ae1a0727 100644
--- a/build/print.go
+++ b/build/print.go
@@ -585,12 +585,12 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		p.margin = m
 
 	case *ParenExpr:
-		p.seq("()", &v.Start, &[]Expr{v.X}, &v.End, modeParen, false, v.ForceMultiLine, false)
+		p.seq("()", &v.Start, &[]Expr{v.X}, &v.End, modeParen, false, v.ForceMultiLine)
 
 	case *CallExpr:
 		addParen(precSuffix)
 		p.expr(v.X, precSuffix)
-		p.seq("()", &v.ListStart, &v.List, &v.End, modeCall, v.ForceCompact, v.ForceMultiLine, false)
+		p.seq("()", &v.ListStart, &v.List, &v.End, modeCall, v.ForceCompact, v.ForceMultiLine)
 
 	case *LoadStmt:
 		addParen(precSuffix)
@@ -615,13 +615,17 @@ func (p *printer) expr(v Expr, outerPrec int) {
 			}
 			args = append(args, arg)
 		}
-		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false, false)
+		p.seq("()", &v.Load, &args, &v.Rparen, modeLoad, v.ForceCompact, false)
 
 	case *ListExpr:
-		p.seq("[]", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, v.FormatAsTable)
+		mode := modeList
+		if v.FormatAsTable {
+			mode = modeTable
+		}
+		p.seq("[]", &v.Start, &v.List, &v.End, mode, false, v.ForceMultiLine)
 
 	case *SetExpr:
-		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine, false)
+		p.seq("{}", &v.Start, &v.List, &v.End, modeList, false, v.ForceMultiLine)
 
 	case *TupleExpr:
 		mode := modeTuple
@@ -632,7 +636,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		if v.FormatAsTableRow {
 			p.tabbedSeq("()", &v.Start, &v.List, &v.End)
 		} else {
-			p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine, false)
+			p.seq("()", &v.Start, &v.List, &v.End, mode, v.ForceCompact, v.ForceMultiLine)
 		}
 
 	case *DictExpr:
@@ -640,7 +644,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 		for _, x := range v.List {
 			list = append(list, x)
 		}
-		p.seq("{}", &v.Start, &list, &v.End, modeDict, false, v.ForceMultiLine, false)
+		p.seq("{}", &v.Start, &list, &v.End, modeDict, false, v.ForceMultiLine)
 
 	case *Comprehension:
 		p.listFor(v)
@@ -663,7 +667,7 @@ func (p *printer) expr(v Expr, outerPrec int) {
 	case *DefStmt:
 		p.printf("def ")
 		p.printf(v.Name)
-		p.seq("()", &v.StartPos, &v.Params, nil, modeDef, v.ForceCompact, v.ForceMultiLine, false)
+		p.seq("()", &v.StartPos, &v.Params, nil, modeDef, v.ForceCompact, v.ForceMultiLine)
 		if v.Type != nil {
 			p.printf(" -> ")
 			p.expr(v.Type, precLow)
@@ -763,6 +767,7 @@ const (
 	modeSeq   // x, y
 	modeDef   // def f(x, y)
 	modeLoad  // load(a, b, c)
+	modeTable // [(x,y)] : List that prints as a table
 )
 
 // useCompactMode reports whether a sequence should be formatted in a compact mode
@@ -778,6 +783,10 @@ func (p *printer) useCompactMode(start *Position, list *[]Expr, end *End, mode s
 		return false
 	}
 
+	// Tables are always multiline
+	if mode == modeTable {
+		return false
+	}
 	// Implicit tuples are always compact
 	if mode == modeSeq {
 		return true
@@ -833,19 +842,19 @@ func (p *printer) useCompactMode(start *Position, list *[]Expr, end *End, mode s
 // The mode parameter specifies the sequence mode (see above).
 // If multiLine is true, seq avoids the compact form even
 // for 0- and 1-element sequences.
-func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mode seqMode, forceCompact, forceMultiLine, forceTabular bool) {
+func (p *printer) seq(brack string, start *Position, list *[]Expr, end *End, mode seqMode, forceCompact, forceMultiLine bool) {
 	if mode != modeSeq {
 		p.printf("%s", brack[:1])
 	}
 
 	p.depth++
-	if forceTabular {
+	if mode == modeTable {
 		p.tabWriterOn = true
 		p.tWriter = new(tabwriter.Writer)
 		p.tWriter.Init(p, 0, 0, 4, ' ', tabwriter.TabIndent)
 	}
 	defer func() {
-		if forceTabular {
+		if mode == modeTable {
 			p.tWriter.Flush()
 		}
 	}()

From bb4a4f51d2dda1bffeb9ed0e70f517117afdc59c Mon Sep 17 00:00:00 2001
From: SKashyap <kshwetha@vmware.com>
Date: Tue, 14 Mar 2023 17:53:55 -0700
Subject: [PATCH 32/32] Handling invalid or missing "column number" for sort.
 Added test cases for the same. Does not rewrite the expression, but functions
 correctly.

---
 build/rewrite.go          | 13 ++++++++-----
 build/testdata/083.golden |  6 ++++++
 build/testdata/083.in     |  6 ++++++
 3 files changed, 20 insertions(+), 5 deletions(-)
 create mode 100644 build/testdata/083.golden
 create mode 100644 build/testdata/083.in

diff --git a/build/rewrite.go b/build/rewrite.go
index 4fcbe27b9..aae7101cd 100644
--- a/build/rewrite.go
+++ b/build/rewrite.go
@@ -124,8 +124,10 @@ func hasComment(x Expr, text string) bool {
 func getColumnNumber(x Expr) int {
 	for _, com := range x.Comment().Before {
 		tokens := strings.Split(strings.ToLower(com.Token), " ")
-		colNum, _ := strconv.Atoi(tokens[4])
-		return colNum
+		colNum, err := strconv.Atoi(tokens[4])
+                if err == nil && colNum > 0 {
+          	   return colNum
+                }
 	}
 	return -1
 }
@@ -439,14 +441,14 @@ func formatTables(f *File) {
 // TODO : Change to in-place sorting later.
 func sortTableRows(v *ListExpr) {
 	// Handle when "#buildifier: table sort N" tag is set
-	if len(v.List) > 0 && tableSort(v.List[0]) > 0 {
+	colNumber := tableSort(v.List[0])
+	if len(v.List) > 0 && colNumber > 0 {
 		// less than 2 rows, nothing to sort
 		if len(v.List) < 2 {
 			return
 		}
 
 		var comments []Comment
-		colNumber := tableSort(v.List[0])
 		keys := make([]string, 0, len(v.List))
 		tableMap := map[string][]*TupleExpr{}
 
@@ -502,7 +504,8 @@ func sortTableRows(v *ListExpr) {
 		}
 		v.List = sortedList
 	}
-}
+ }
+
 
 // sortStringLists sorts lists of string literals used as specific rule arguments.
 func sortStringLists(f *File) {
diff --git a/build/testdata/083.golden b/build/testdata/083.golden
new file mode 100644
index 000000000..99c18935c
--- /dev/null
+++ b/build/testdata/083.golden
@@ -0,0 +1,6 @@
+manifest = [
+    # buildifier: table sort -1
+    ("ab",    "bc",    "cd"),
+    ("a",     "b",     "c"),
+    ("",      "",      ""),
+]
diff --git a/build/testdata/083.in b/build/testdata/083.in
new file mode 100644
index 000000000..e20a1b389
--- /dev/null
+++ b/build/testdata/083.in
@@ -0,0 +1,6 @@
+manifest = [
+   # buildifier: table sort -1
+   ("ab", "bc", "cd"),
+   ("a",  "b",  "c"),
+   ("",   "",    ""),
+]