Repository: benhoyt/goawk
Pull request #153 (2022-09-22 21:06:21)

Better filenames tracking
This is the other PR in context of #144.

This is re-implemenation of #150 based on work in #151. I think #151 can be just closed.

Basically this is almost equivalent to #151 plus:
1. added tests
2. added doc comments
3. applied minor optimization

xonixx: @benhoyt Any thoughts?
benhoyt: Yes, sorry, last week was a busy week. I'll try to get to this tonight or tomorrow.
xonixx: Done. Please check if it's ok now.

Patch:
diff --git a/goawk.go b/goawk.go
index dbdb130..85be5ee 100644
--- a/goawk.go
+++ b/goawk.go
@@ -30,7 +30,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -38,6 +37,7 @@ import (
 	"strings"
 	"unicode/utf8"
 
+	"github.com/benhoyt/goawk/internal/parseutil"
 	"github.com/benhoyt/goawk/interp"
 	"github.com/benhoyt/goawk/lexer"
 	"github.com/benhoyt/goawk/parser"
@@ -194,41 +194,37 @@ argsLoop:
 	// Any remaining args are program and input files
 	args := os.Args[i:]
 
-	var src []byte
-	var stdinBytes []byte // used if there's a parse error
+	fileReader := &parseutil.FileReader{}
 	if len(progFiles) > 0 {
 		// Read source: the concatenation of all source files specified
-		buf := &bytes.Buffer{}
 		progFiles = expandWildcardsOnWindows(progFiles)
 		for _, progFile := range progFiles {
 			if progFile == "-" {
-				b, err := ioutil.ReadAll(os.Stdin)
+				err := fileReader.AddFile("<stdin>", os.Stdin)
 				if err != nil {
 					errorExit(err)
 				}
-				stdinBytes = b
-				_, _ = buf.Write(b)
 			} else {
 				f, err := os.Open(progFile)
 				if err != nil {
 					errorExit(err)
 				}
-				_, err = buf.ReadFrom(f)
+				err = fileReader.AddFile(progFile, f)
 				if err != nil {
 					_ = f.Close()
 					errorExit(err)
 				}
 				_ = f.Close()
 			}
-			// Append newline to file in case it doesn't end with one
-			_ = buf.WriteByte('\n')
 		}
-		src = buf.Bytes()
 	} else {
 		if len(args) < 1 {
 			errorExitf(shortUsage)
 		}
-		src = []byte(args[0])
+		err := fileReader.AddFile("<cmdline>", strings.NewReader(args[0]))
+		if err != nil {
+			errorExit(err)
+		}
 		args = args[1:]
 	}
 
@@ -237,13 +233,13 @@ argsLoop:
 		DebugTypes:  debugTypes,
 		DebugWriter: os.Stderr,
 	}
-	prog, err := parser.ParseProgram(src, parserConfig)
+	prog, err := parser.ParseProgram(fileReader.Source(), parserConfig)
 	if err != nil {
 		if err, ok := err.(*parser.ParseError); ok {
-			name, line := errorFileLine(progFiles, stdinBytes, err.Position.Line)
+			name, line := fileReader.FileLine(err.Position.Line)
 			fmt.Fprintf(os.Stderr, "%s:%d:%d: %s\n",
 				name, line, err.Position.Column, err.Message)
-			showSourceLine(src, err.Position)
+			showSourceLine(fileReader.Source(), err.Position)
 			os.Exit(1)
 		}
 		errorExitf("%s", err)
@@ -347,36 +343,6 @@ func showSourceLine(src []byte, pos lexer.Position) {
 	fmt.Fprintln(os.Stderr, strings.Repeat(" ", runeColumn)+strings.Repeat("   ", numTabs)+"^")
 }
 
-// Determine which filename and line number to display for the overall
-// error line number.
-func errorFileLine(progFiles []string, stdinBytes []byte, errorLine int) (string, int) {
-	if len(progFiles) == 0 {
-		return "<cmdline>", errorLine
-	}
-	startLine := 1
-	for _, progFile := range progFiles {
-		var content []byte
-		if progFile == "-" {
-			progFile = "<stdin>"
-			content = stdinBytes
-		} else {
-			b, err := ioutil.ReadFile(progFile)
-			if err != nil {
-				return "<unknown>", errorLine
-			}
-			content = b
-		}
-		content = append(content, '\n')
-
-		numLines := bytes.Count(content, []byte{'\n'})
-		if errorLine >= startLine && errorLine < startLine+numLines {
-			return progFile, errorLine - startLine + 1
-		}
-		startLine += numLines
-	}
-	return "<unknown>", errorLine
-}
-
 func errorExit(err error) {
 	pathErr, ok := err.(*os.PathError)
 	if ok && os.IsNotExist(err) {
diff --git a/internal/parseutil/filereader.go b/internal/parseutil/filereader.go
new file mode 100644
index 0000000..8ab2bb6
--- /dev/null
+++ b/internal/parseutil/filereader.go
@@ -0,0 +1,55 @@
+// Package parseutil contains various utilities for parsing GoAWK source code.
+package parseutil
+
+import (
+	"bytes"
+	"io"
+)
+
+// FileReader serves two purposes:
+// 1. read input sources and join them into a single source (slice of bytes)
+// 2. track the lines counts of each input source
+type FileReader struct {
+	files  []file
+	source bytes.Buffer
+}
+
+type file struct {
+	path  string
+	lines int
+}
+
+// AddFile adds a single source file.
+func (fr *FileReader) AddFile(path string, source io.Reader) error {
+	curLen := fr.source.Len()
+	_, err := fr.source.ReadFrom(source)
+	if err != nil {
+		return err
+	}
+	if !bytes.HasSuffix(fr.source.Bytes(), []byte("\n")) {
+		// Append newline to file in case it doesn't end with one
+		fr.source.WriteByte('\n')
+	}
+	content := fr.source.Bytes()[curLen:]
+	lines := bytes.Count(content, []byte("\n"))
+	fr.files = append(fr.files, file{path, lines})
+	return nil
+}
+
+// FileLine resolves an overall line number from the concatenated source code
+// to the local line number in that source file (identified by path).
+func (fr *FileReader) FileLine(line int) (path string, fileLine int) {
+	startLine := 1
+	for _, f := range fr.files {
+		if line >= startLine && line < startLine+f.lines {
+			return f.path, line - startLine + 1
+		}
+		startLine += f.lines
+	}
+	return "", 0
+}
+
+// Source returns the concatenated source code from all files added.
+func (fr *FileReader) Source() []byte {
+	return fr.source.Bytes()
+}


Test patch:
diff --git a/internal/parseutil/filereader_test.go b/internal/parseutil/filereader_test.go
new file mode 100644
index 0000000..24c13ad
--- /dev/null
+++ b/internal/parseutil/filereader_test.go
@@ -0,0 +1,138 @@
+package parseutil_test
+
+import (
+	. "github.com/benhoyt/goawk/internal/parseutil"
+	"strings"
+	"testing"
+)
+
+type testFile struct{ name, source string }
+
+type test struct {
+	name string
+	// input:
+	files []testFile
+	line  int
+	// expected:
+	path     string
+	fileLine int
+}
+
+func TestFileReader(t *testing.T) {
+	fileSetNoNewline := []testFile{
+		{"file1", `BEGIN {
+print f(1)
+}`},
+		{"file2", `function f(x) {
+print x
+}`},
+	}
+	fileSetWithNewline := []testFile{
+		{"file1", `BEGIN {
+print f(1)
+}
+`},
+		{"file2", `function f(x) {
+print x
+}
+`},
+	}
+	tests := []test{
+		{
+			"TestInFirstFile",
+			fileSetNoNewline,
+			2,
+			"file1",
+			2,
+		},
+		{
+			"TestInSecondFile",
+			fileSetNoNewline,
+			5,
+			"file2",
+			2,
+		},
+		{
+			"TestInFirstFileWithNewline",
+			fileSetWithNewline,
+			2,
+			"file1",
+			2,
+		},
+		{
+			"TestInSecondFileWithNewline",
+			fileSetWithNewline,
+			5,
+			"file2",
+			2,
+		},
+		{
+			"TestOutside",
+			fileSetNoNewline,
+			100,
+			"",
+			0,
+		},
+		{
+			"TestOutsideNegative",
+			fileSetNoNewline,
+			-100,
+			"",
+			0,
+		},
+		{
+			"TestNoFiles",
+			[]testFile{},
+			1,
+			"",
+			0,
+		},
+		{
+			"TestZeroLenFiles",
+			[]testFile{
+				{"file1", ""},
+			},
+			1,
+			"file1",
+			1,
+		},
+		{
+			"TestZeroLenFiles1",
+			[]testFile{
+				{"file1", ""},
+			},
+			2,
+			"",
+			0,
+		},
+	}
+
+	for _, tst := range tests {
+		t.Run(tst.name, func(t *testing.T) {
+
+			fr := &FileReader{}
+
+			for _, file := range tst.files {
+				if nil != fr.AddFile(file.name, strings.NewReader(file.source)) {
+					panic("should not happen")
+				}
+			}
+
+			path, fileLine := fr.FileLine(tst.line)
+			if path != tst.path {
+				t.Errorf("expected path: %v, got: %v", tst.path, path)
+			}
+			if fileLine != tst.fileLine {
+				t.Errorf("expected fileLine: %v, got: %v", tst.fileLine, fileLine)
+			}
+
+			// test result source
+			source := string(fr.Source())
+			for _, file := range tst.files {
+				if strings.Index(source, file.source) < 0 {
+					t.Errorf("Source() is incorrect")
+				}
+			}
+		})
+	}
+}