Skip to content

Commit

Permalink
cue/token: deprecate base offset
Browse files Browse the repository at this point in the history
The base offset seems to be a hangover from the origins of the
package in [`go/token`](https://pkg.go.dev/go/token#FileSet.AddFile).
In that package, it signifies the overall offset inside a set of
line information shared amongst between many files.  In `cue/token`,
by contrast, it doesn't signify anything, witnessed by the fact
that when we _do_ try to use it, it [doesn't have the desired
effect](#2776).

In effect, the base is added to the internal index stored in a
`token.Pos` and then removed before it's made visible to the API.

Because it's confusing, and because in general we can't
say that we're starting at an arbitrary point in a file
and hope line and column information will be able to work,
we deprecate the API, maintaining all user-visible behavior.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I57d0f54722bf4aa872ef1f36e2d2a8e2e43a5810
  • Loading branch information
rogpeppe committed Jan 22, 2024
1 parent b55e471 commit 910f008
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 46 deletions.
2 changes: 1 addition & 1 deletion cue/parser/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func checkErrors(t *testing.T, filename string, input interface{}) {
// we are expecting the following errors
// (collect these after parsing a file so that it is found in the file set)
if file == nil {
t.Fatal("")
t.Fatal("no token.File for ast.File")
}
expected := expectedErrors(t, file, src)

Expand Down
4 changes: 3 additions & 1 deletion cue/parser/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,10 @@ const (
)

// FileOffset specifies the File position info to use.
//
// Deprecated: this has no effect.
func FileOffset(pos int) Option {
return func(p *parser) { p.offset = pos }
return func(p *parser) {}
}

// A mode value is a set of flags (or 0).
Expand Down
4 changes: 1 addition & 3 deletions cue/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ var debugStr = astinternal.DebugStr
// The parser structure holds the parser's internal state.
type parser struct {
file *token.File
offset int
errors errors.Error
scanner scanner.Scanner

Expand Down Expand Up @@ -68,11 +67,10 @@ type parser struct {
}

func (p *parser) init(filename string, src []byte, mode []Option) {
p.offset = -1
for _, f := range mode {
f(p)
}
p.file = token.NewFile(filename, p.offset, len(src))
p.file = token.NewFile(filename, -1, len(src))

var m scanner.Mode
if p.mode&parseCommentsMode != 0 {
Expand Down
2 changes: 1 addition & 1 deletion cue/scanner/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func Fuzz(b []byte) int {
}

var s Scanner
s.Init(token.NewFile("", 1, len(b)), b, eh, ScanComments)
s.Init(token.NewFile("", -1, len(b)), b, eh, ScanComments)

for {
_, tok, _ := s.Scan()
Expand Down
24 changes: 12 additions & 12 deletions cue/scanner/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func TestScan(t *testing.T) {

// verify scan
var s Scanner
s.Init(token.NewFile("", 1, len(source)), source, eh, ScanComments|DontInsertCommas)
s.Init(token.NewFile("", -1, len(source)), source, eh, ScanComments|DontInsertCommas)

// set up expected position
epos := token.Position{
Expand Down Expand Up @@ -327,7 +327,7 @@ func TestScan(t *testing.T) {

func checkComma(t *testing.T, line string, mode Mode) {
var S Scanner
file := token.NewFile("TestCommas", 1, len(line))
file := token.NewFile("TestCommas", -1, len(line))
S.Init(file, []byte(line), nil, mode)
pos, tok, lit := S.Scan()
for tok != token.EOF {
Expand Down Expand Up @@ -496,7 +496,7 @@ func TestRelative(t *testing.T) {
"elided , \n",
}
var S Scanner
f := token.NewFile("TestCommas", 1, len(test))
f := token.NewFile("TestCommas", -1, len(test))
S.Init(f, []byte(test), nil, ScanComments)
pos, tok, lit := S.Scan()
got := []string{}
Expand Down Expand Up @@ -558,7 +558,7 @@ func TestLineComments(t *testing.T) {

// verify scan
var S Scanner
f := token.NewFile(filepath.Join("dir", "TestLineComments"), 1, len(src))
f := token.NewFile(filepath.Join("dir", "TestLineComments"), -1, len(src))
S.Init(f, []byte(src), nil, DontInsertCommas)
for _, s := range segs {
p, _, lit := S.Scan()
Expand All @@ -582,7 +582,7 @@ func TestInit(t *testing.T) {

// 1st init
src1 := "false true { }"
f1 := token.NewFile("src1", 1, len(src1))
f1 := token.NewFile("src1", -1, len(src1))
s.Init(f1, []byte(src1), nil, DontInsertCommas)
if f1.Size() != len(src1) {
t.Errorf("bad file size: got %d, expected %d", f1.Size(), len(src1))
Expand All @@ -596,7 +596,7 @@ func TestInit(t *testing.T) {

// 2nd init
src2 := "null true { ]"
f2 := token.NewFile("src2", 1, len(src2))
f2 := token.NewFile("src2", -1, len(src2))
s.Init(f2, []byte(src2), nil, DontInsertCommas)
if f2.Size() != len(src2) {
t.Errorf("bad file size: got %d, expected %d", f2.Size(), len(src2))
Expand Down Expand Up @@ -629,7 +629,7 @@ func TestScanInterpolation(t *testing.T) {
for i, src := range sources {
name := fmt.Sprintf("tsrc%d", i)
t.Run(name, func(t *testing.T) {
f := token.NewFile(name, 1, len(src))
f := token.NewFile(name, -1, len(src))

// verify scan
var s Scanner
Expand Down Expand Up @@ -674,7 +674,7 @@ func TestStdErrorHander(t *testing.T) {
}

var s Scanner
s.Init(token.NewFile("File1", 1, len(src)), []byte(src), eh, DontInsertCommas)
s.Init(token.NewFile("File1", -1, len(src)), []byte(src), eh, DontInsertCommas)
for {
if _, tok, _ := s.Scan(); tok == token.EOF {
break
Expand Down Expand Up @@ -713,7 +713,7 @@ func checkError(t *testing.T, src string, tok token.Token, pos int, lit, err str
h.msg = fmt.Sprintf(msg, args...)
h.pos = pos
}
s.Init(token.NewFile("", 1, len(src)), []byte(src), eh, ScanComments|DontInsertCommas)
s.Init(token.NewFile("", -1, len(src)), []byte(src), eh, ScanComments|DontInsertCommas)
_, tok0, lit0 := s.Scan()
if tok0 != tok {
t.Errorf("%q: got %s, expected %s", src, tok0, tok)
Expand Down Expand Up @@ -861,7 +861,7 @@ func TestNoLiteralComments(t *testing.T) {
}
`
var s Scanner
s.Init(token.NewFile("", 1, len(src)), []byte(src), nil, 0)
s.Init(token.NewFile("", -1, len(src)), []byte(src), nil, 0)
for {
pos, tok, lit := s.Scan()
class := tokenclass(tok)
Expand All @@ -876,7 +876,7 @@ func TestNoLiteralComments(t *testing.T) {

func BenchmarkScan(b *testing.B) {
b.StopTimer()
file := token.NewFile("", 1, len(source))
file := token.NewFile("", -1, len(source))
var s Scanner
b.StartTimer()
for i := 0; i < b.N; i++ {
Expand All @@ -897,7 +897,7 @@ func BenchmarkScanFile(b *testing.B) {
if err != nil {
panic(err)
}
file := token.NewFile(filename, 1, len(src))
file := token.NewFile(filename, -1, len(src))
b.SetBytes(int64(len(src)))
var s Scanner
b.StartTimer()
Expand Down
37 changes: 24 additions & 13 deletions cue/token/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,39 +199,50 @@ func toPos(x index) int {
// -----------------------------------------------------------------------------
// File

// index represents an offset into the file.
// It's 1-based rather than zero-based so that
// we can distinguish the zero Pos from a Pos that
// just has a zero offset.
type index int

// A File has a name, size, and line offset table.
type File struct {
mutex sync.RWMutex
name string // file name as provided to AddFile
base index // Pos index range for this file is [base...base+size]
size index // file size as provided to AddFile
// base is deprecated and stored only so that [File.Base]
// can continue to return the same value passed to [NewFile].
base index

// lines and infos are protected by set.mutex
lines []index // lines contains the offset of the first character for each line (the first entry is always 0)
infos []lineInfo
}

// NewFile returns a new file.
func NewFile(filename string, base, size int) *File {
if base < 0 {
base = 1
// NewFile returns a new file with the given OS file name. The size provides the
// size of the whole file.
//
// The second argument is deprecated. It has no effect.
func NewFile(filename string, deprecatedBase, size int) *File {
if deprecatedBase < 0 {
deprecatedBase = 1
}
return &File{sync.RWMutex{}, filename, index(base), index(size), []index{0}, nil}
return &File{sync.RWMutex{}, nil, filename, index(size), index(deprecatedBase), []index{0}, nil}
}

// Name returns the file name of file f as registered with AddFile.
func (f *File) Name() string {
return f.name
}

// Base returns the base offset of file f as registered with AddFile.
// Base returns the base offset of file f as passed to NewFile.
//
// Deprecated: this method just returns the (deprecated) second argument passed to NewFile.
func (f *File) Base() int {
return int(f.base)
}

// Size returns the size of file f as registered with AddFile.
// Size returns the size of file f as passed to NewFile.
func (f *File) Size() int {
return int(f.size)
}
Expand Down Expand Up @@ -359,18 +370,18 @@ func (f *File) Pos(offset int, rel RelPos) Pos {
if index(offset) > f.size {
panic("illegal file offset")
}
return Pos{f, toPos(f.base+index(offset)) + int(rel)}
return Pos{f, toPos(1+index(offset)) + int(rel)}
}

// Offset returns the offset for the given file position p;
// p must be a valid Pos value in that file.
// f.Offset(f.Pos(offset)) == offset.
func (f *File) Offset(p Pos) int {
x := p.index()
if x < f.base || x > f.base+index(f.size) {
if x < 1 || x > 1+index(f.size) {
panic("illegal Pos value")
}
return int(x - f.base)
return int(x - 1)
}

// Line returns the line number for the given file position p;
Expand Down Expand Up @@ -405,7 +416,7 @@ func (f *File) unpack(offset index, adjusted bool) (filename string, line, colum
}

func (f *File) position(p Pos, adjusted bool) (pos Position) {
offset := p.index() - f.base
offset := p.index() - 1
pos.Offset = int(offset)
pos.Filename, pos.Line, pos.Column = f.unpack(offset, adjusted)
return
Expand All @@ -418,7 +429,7 @@ func (f *File) position(p Pos, adjusted bool) (pos Position) {
func (f *File) PositionFor(p Pos, adjusted bool) (pos Position) {
x := p.index()
if p != NoPos {
if x < f.base || x > f.base+f.size {
if x < 1 || x > 1+f.size {
panic("illegal Pos value")
}
pos = f.position(p, adjusted)
Expand Down
21 changes: 9 additions & 12 deletions encoding/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,17 @@ func extract(path string, b []byte) (ast.Expr, error) {
// The runtime may be nil if Decode isn't used.
func NewDecoder(r *cue.Runtime, path string, src io.Reader) *Decoder {
return &Decoder{
r: r,
path: path,
dec: json.NewDecoder(src),
offset: 1,
r: r,
path: path,
dec: json.NewDecoder(src),
}
}

// A Decoder converts JSON values to CUE.
type Decoder struct {
r *cue.Runtime
path string
dec *json.Decoder
offset int
r *cue.Runtime
path string
dec *json.Decoder
}

// Extract converts the current JSON value to a CUE ast. It returns io.EOF
Expand All @@ -131,13 +129,12 @@ func (d *Decoder) extract() (ast.Expr, error) {
if err == io.EOF {
return nil, err
}
offset := d.offset
d.offset += len(raw)
if err != nil {
pos := token.NewFile(d.path, offset, len(raw)).Pos(0, 0)
pos := token.NewFile(d.path, -1, len(raw)).Pos(0, 0)
return nil, errors.Wrapf(err, pos, "invalid JSON for file %q", d.path)
}
expr, err := parser.ParseExpr(d.path, []byte(raw), parser.FileOffset(offset))
expr, err := parser.ParseExpr(d.path, []byte(raw))

if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion encoding/protobuf/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (s *Extractor) parse(filename string, src interface{}) (p *protoConverter,
return nil, errors.Newf(token.NoPos, "protobuf: %v", err)
}

tfile := token.NewFile(filename, 0, len(b))
tfile := token.NewFile(filename, -1, len(b))
tfile.SetLinesForContent(b)

p = &protoConverter{
Expand Down
2 changes: 1 addition & 1 deletion encoding/protobuf/textproto/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (d *Decoder) Parse(schema cue.Value, filename string, b []byte) (ast.Expr,

// dec.errs = nil

f := token.NewFile(filename, 0, len(b))
f := token.NewFile(filename, -1, len(b))
f.SetLinesForContent(b)
dec.file = f

Expand Down
2 changes: 1 addition & 1 deletion internal/attrs.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func ParseAttrBody(pos token.Pos, s string) (a Attr) {
// Create temporary token.File so that scanner has something
// to work with.
// TODO it's probably possible to do this without allocations.
tmpFile := token.NewFile("", 0, len(s))
tmpFile := token.NewFile("", -1, len(s))
if len(s) > 0 {
tmpFile.AddLine(len(s) - 1)
}
Expand Down

0 comments on commit 910f008

Please sign in to comment.