this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

internal: replace ParseAttrBody with ParseAttr

Currently ParseAttrBody is the only way of creating
an internal.Attr, but it doesn't populate the Name
field because it doesn't have access to it.

It turns out that everywhere that is currently using ParseAttrBody
can also use slightly higher level function that takes
an ast.Attribute instead. This can usefully populate the
Name and Pos fields from the AST node,
which removes the need for second-guessing of
the position logic (as currently done in the cue package).

There are some places which don't have an ast.Attribute
already, but they're only in tests and it's easy to
construct one for the purpose.

We make ParseAttr return a pointer because it's
conventional in Go that the constructor for a type
with all pointer methods returns a pointer. There's
a small potential performance overhead to this change
but this should be negligible compared to what's going
on inside the ParseAttr logic itself.

The only wrinkle is that most of the places that currently
call ParseAttrBody invoke Attribute.Split first to find the
attribute name. We add an ast.Attribute.Name method
to make this inspection trivial. This also implies a small
amount of additional runtime overhead, but it should be
lost in the noise: scanning a few bytes is cheap and
even cheaper when we've done exactly the same scan
a few instructions previously. I think that the additional
simplicity makes this worthwhile.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I23eb0e3f997ef520a9a8e3d5abe8b9afde3121dc
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1235347
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Matthew Sackman <matthew@cue.works>

+92 -91
+6 -1
cue/ast/ast.go
··· 303 303 func (a *Attribute) pos() *token.Pos { return &a.At } 304 304 func (a *Attribute) End() token.Pos { return a.At.Add(len(a.Text)) } 305 305 306 - func (a *Attribute) Split() (key, body string) { 306 + func (a *Attribute) Name() string { 307 + name, _ := a.Split() 308 + return name 309 + } 310 + 311 + func (a *Attribute) Split() (name, body string) { 307 312 s := a.Text 308 313 p := strings.IndexByte(s, '(') 309 314 if p < 0 || !strings.HasPrefix(s, "@") || !strings.HasSuffix(s, ")") {
+1 -8
cue/attribute.go
··· 44 44 } 45 45 46 46 func newAttr(k internal.AttrKind, a *ast.Attribute) Attribute { 47 - key, body := a.Split() 48 - // Note: the body is always positioned just after 49 - // the opening ( after the key. 50 - x := internal.ParseAttrBody(a.Pos().Add(len(key)+1), body) 51 - x.Name = key 52 - x.Kind = k 53 - x.Pos = a.Pos() 54 - return Attribute{x} 47 + return Attribute{*internal.ParseAttr(a)} 55 48 } 56 49 57 50 func nonExistAttr(key string) Attribute {
+7 -8
cue/load/tags.go
··· 169 169 field *ast.Field 170 170 } 171 171 172 - func parseTag(pos token.Pos, body string) (t *tag, err errors.Error) { 172 + func parseTag(astAttr *ast.Attribute) (t *tag, err errors.Error) { 173 173 t = &tag{} 174 174 t.kind = cue.StringKind 175 175 176 - a := internal.ParseAttrBody(pos, body) 176 + a := internal.ParseAttr(astAttr) 177 177 178 178 t.key, _ = a.String(0) 179 179 if !ast.IsValidIdent(t.key) { 180 - return t, errors.Newf(pos, "invalid identifier %q", t.key) 180 + return t, errors.Newf(a.Pos, "invalid identifier %q", t.key) 181 181 } 182 182 183 183 if s, ok, _ := a.Lookup(1, "type"); ok { ··· 190 190 case "bool": 191 191 t.kind = cue.BoolKind 192 192 default: 193 - return t, errors.Newf(pos, "invalid type %q", s) 193 + return t, errors.Newf(a.Pos, "invalid type %q", s) 194 194 } 195 195 } 196 196 197 197 if s, ok, _ := a.Lookup(1, "short"); ok { 198 198 for s := range strings.SplitSeq(s, "|") { 199 199 if !ast.IsValidIdent(t.key) { 200 - return t, errors.Newf(pos, "invalid identifier %q", s) 200 + return t, errors.Newf(a.Pos, "invalid identifier %q", s) 201 201 } 202 202 t.shorthands = append(t.shorthands, s) 203 203 } ··· 265 265 } 266 266 267 267 for _, a := range x.Attrs { 268 - key, body := a.Split() 269 - if key != "tag" { 268 + if a.Name() != "tag" { 270 269 continue 271 270 } 272 - t, err := parseTag(a.Pos(), body) 271 + t, err := parseTag(a) 273 272 if err != nil { 274 273 errs = errors.Append(errs, err) 275 274 continue
+25 -13
internal/attrs.go
··· 18 18 "fmt" 19 19 "strings" 20 20 21 + "cuelang.org/go/cue/ast" 21 22 "cuelang.org/go/cue/errors" 22 23 "cuelang.org/go/cue/literal" 23 24 "cuelang.org/go/cue/scanner" ··· 155 156 return "", false, nil 156 157 } 157 158 158 - func ParseAttrBody(pos token.Pos, s string) (a Attr) { 159 + // ParseAttr parses the given attribute. It always returns a non-nil 160 + // [Attr], which will have a non-nil Err field if there's an error. 161 + func ParseAttr(astAttr *ast.Attribute) *Attr { 162 + key, body := astAttr.Split() 163 + 164 + // Note: the body is always positioned just after the opening ( 165 + // after the key 166 + pos := astAttr.Pos().Add(len(key) + 1) 167 + 159 168 // Create temporary token.File so that scanner has something 160 169 // to work with. 161 170 // TODO it's probably possible to do this without allocations. 162 - tmpFile := token.NewFile("", -1, len(s)) 163 - if len(s) > 0 { 164 - tmpFile.AddLine(len(s) - 1) 171 + tmpFile := token.NewFile("", -1, len(body)) 172 + if len(body) > 0 { 173 + tmpFile.AddLine(len(body) - 1) 174 + } 175 + a := &Attr{ 176 + Pos: astAttr.Pos(), 177 + Name: key, 178 + Body: body, 165 179 } 166 - a.Body = s 167 - a.Pos = pos 168 180 var scan scanner.Scanner 169 - scan.Init(tmpFile, []byte(s), nil, scanner.DontInsertCommas) 181 + scan.Init(tmpFile, []byte(body), nil, scanner.DontInsertCommas) 170 182 for { 171 183 start := scan.Offset() 172 184 tok, err := scanAttributeTokens(&scan, pos, 1<<token.COMMA|1<<token.BIND|1<<token.EOF) ··· 179 191 switch tok { 180 192 case token.EOF: 181 193 // Empty field. 182 - a.appendField("", s[start:], s[start:]) 194 + a.appendField("", body[start:], body[start:]) 183 195 return a 184 196 case token.COMMA: 185 - val := s[start : scan.Offset()-1] 197 + val := body[start : scan.Offset()-1] 186 198 a.appendField("", val, val) // All but final comma. 187 199 continue 188 200 } 189 201 valStart := scan.Offset() 190 - key := s[start : valStart-1] // All but =. 202 + key := body[start : valStart-1] // All but =. 191 203 tok, err = scanAttributeTokens(&scan, pos, 1<<token.COMMA|1<<token.EOF) 192 204 if err != nil { 193 205 // Shouldn't happen because bracket nesting should have been checked previously by ··· 195 207 a.Err = err 196 208 return a 197 209 } 198 - valEnd := len(s) 210 + valEnd := len(body) 199 211 if tok != token.EOF { 200 212 valEnd = scan.Offset() - 1 // All but final comma 201 213 } 202 - value := s[valStart:valEnd] 203 - text := s[start:valEnd] 214 + value := body[valStart:valEnd] 215 + text := body[start:valEnd] 204 216 a.appendField(key, value, text) 205 217 if tok == token.EOF { 206 218 return a
+10 -15
internal/attrs_test.go
··· 16 16 17 17 import ( 18 18 "fmt" 19 - "strings" 20 19 "testing" 21 20 21 + "cuelang.org/go/cue/ast" 22 22 "cuelang.org/go/cue/token" 23 + "github.com/go-quicktest/qt" 23 24 "github.com/google/go-cmp/cmp" 24 25 ) 25 26 26 27 type keyVals [][3]string 27 28 28 - func TestAttributeBody(t *testing.T) { 29 + func TestParseAttr(t *testing.T) { 29 30 testdata := []struct { 30 31 in string 31 32 out keyVals ··· 103 104 for i, tc := range testdata { 104 105 t.Run(fmt.Sprintf("%d-%s", i, tc.in), func(t *testing.T) { 105 106 f := token.NewFile("test", -1, len(tc.in)) 106 - pos := f.Pos(0, token.NoRelPos) 107 - pa := ParseAttrBody(pos, tc.in) 108 - err := pa.Err 109 - 107 + pa := ParseAttr(&ast.Attribute{ 108 + At: f.Pos(0, token.NoRelPos), 109 + Text: "@attr(" + tc.in + ")", 110 + }) 111 + qt.Assert(t, qt.Equals(pa.Name, "attr")) 110 112 if tc.err != "" { 111 - if err == nil { 112 - t.Fatalf("unexpected success when error was expected (%#v)", pa.Fields) 113 - } 114 - if !strings.Contains(err.Error(), tc.err) { 115 - t.Errorf("error was %v; want %v", err, tc.err) 116 - } 113 + qt.Assert(t, qt.ErrorMatches(pa.Err, tc.err)) 117 114 return 118 115 } 119 - if err != nil { 120 - t.Fatal(err) 121 - } 116 + qt.Assert(t, qt.IsNil(pa.Err)) 122 117 var kvs keyVals 123 118 for _, kv := range pa.Fields { 124 119 kvs = append(kvs, [3]string{kv.Key(), kv.Value(), kv.Text()})
+22 -26
internal/core/runtime/extern.go
··· 131 131 var ( 132 132 hasPkg bool 133 133 p int 134 - fileAttrs []*ast.Attribute 134 + fileAttrs []token.Pos 135 135 ) 136 136 137 137 loop: ··· 142 142 break loop 143 143 144 144 case *ast.Attribute: 145 - pos := a.Pos() 146 - key, body := a.Split() 147 - if key != "extern" { 145 + if a.Name() != "extern" { 148 146 continue 149 147 } 150 - fileAttrs = append(fileAttrs, a) 151 - 152 - attr := internal.ParseAttrBody(pos, body) 148 + attr := internal.ParseAttr(a) 149 + fileAttrs = append(fileAttrs, attr.Pos) 153 150 if attr.Err != nil { 154 151 err = errors.Append(err, attr.Err) 155 152 continue 156 153 } 154 + 157 155 k, e := attr.String(0) 158 156 if e != nil { 159 157 // Unreachable. 160 - err = errors.Append(err, errors.Newf(pos, "%s", e)) 158 + err = errors.Append(err, errors.Newf(attr.Pos, "%s", e)) 161 159 continue 162 160 } 163 161 164 162 if k == "" { 165 - err = errors.Append(err, errors.Newf(pos, 163 + err = errors.Append(err, errors.Newf(attr.Pos, 166 164 "interpreter name must be non-empty")) 167 165 continue 168 166 } ··· 171 169 kinds = map[string]token.Pos{} 172 170 } 173 171 if _, ok := kinds[k]; ok { 174 - err = errors.Append(err, errors.Newf(pos, 172 + err = errors.Append(err, errors.Newf(attr.Pos, 175 173 "duplicate @extern attribute for kind %q", k)) 176 174 continue 177 175 } 178 - kinds[k] = pos 176 + kinds[k] = attr.Pos 179 177 } 180 178 } 181 179 ··· 185 183 186 184 case len(fileAttrs) > 0 && !hasPkg: 187 185 for _, a := range fileAttrs { 188 - err = errors.Append(err, errors.Newf(a.Pos(), 186 + err = errors.Append(err, errors.Newf(a, 189 187 "extern attribute without package clause")) 190 188 } 191 189 return nil, nil, err ··· 252 250 nodeStack = append(nodeStack, n.Value) 253 251 254 252 case *ast.Attribute: 255 - k, body := n.Split() 256 - if k != kind { 253 + if n.Name() != kind { 257 254 break 258 255 } 259 256 260 - attrParsed := internal.ParseAttrBody(n.Pos(), body) 257 + attrParsed := internal.ParseAttr(n) 261 258 parent := nodeStack[len(nodeStack)-1] 262 259 if attrsByNode == nil { 263 260 attrsByNode = make(map[ast.Node][]*internal.Attr) 264 261 } 265 - attrsByNode[parent] = append(attrsByNode[parent], &attrParsed) 262 + attrsByNode[parent] = append(attrsByNode[parent], attrParsed) 266 263 return false 267 264 } 268 265 ··· 348 345 } 349 346 } 350 347 351 - func (d *externDecorator) externValue(attr *ast.Attribute, name string, kinds map[string]bool, scope *adt.Vertex) adt.Expr { 352 - kind, body := attr.Split() 353 - if !kinds[kind] { 348 + func (d *externDecorator) externValue(astAttr *ast.Attribute, name string, kinds map[string]bool, scope *adt.Vertex) adt.Expr { 349 + if !kinds[astAttr.Name()] { 354 350 return nil 355 351 } 356 - parsed := internal.ParseAttrBody(attr.Pos(), body) 357 - if parsed.Err != nil { 358 - d.errs = errors.Append(d.errs, parsed.Err) 352 + attr := internal.ParseAttr(astAttr) 353 + if attr.Err != nil { 354 + d.errs = errors.Append(d.errs, attr.Err) 359 355 return nil 360 356 } 361 - c := d.compilers[kind] 357 + c := d.compilers[attr.Name] 362 358 if c == nil { 363 359 return nil 364 360 } 365 - if a, ok, _ := parsed.Lookup(1, "name"); ok { 361 + if a, ok, _ := attr.Lookup(1, "name"); ok { 366 362 name = a 367 363 } 368 - b, err := c.Compile(name, scope, &parsed) 364 + b, err := c.Compile(name, scope, attr) 369 365 if err != nil { 370 - d.errs = errors.Append(d.errs, errors.Wrap(errors.Newf(attr.Pos(), "@%s", kind), err)) 366 + d.errs = errors.Append(d.errs, errors.Wrap(errors.Newf(attr.Pos, "@%s", attr.Name), err)) 371 367 return nil 372 368 } 373 369 return b
+14 -15
internal/cuetxtar/inline.go
··· 151 151 version string 152 152 153 153 // raw is the parsed internal.Attr for accessing remaining arguments. 154 - raw internal.Attr 154 + raw *internal.Attr 155 155 156 156 // For "err" directives, parsed sub-options are stored here. 157 157 errArgs *errArgs ··· 197 197 // It returns a parsedTestAttr for each logical directive in the attribute. 198 198 // A single @test(...) contains exactly one directive (the first positional 199 199 // argument or the key of the first key=value pair). 200 - func parseTestAttr(a *ast.Attribute) (parsedTestAttr, error) { 201 - key, body := a.Split() 202 - if key != "test" { 203 - return parsedTestAttr{}, fmt.Errorf("not a @test attribute: @%s", key) 200 + func parseTestAttr(astAttr *ast.Attribute) (parsedTestAttr, error) { 201 + if name := astAttr.Name(); name != "test" { 202 + return parsedTestAttr{}, fmt.Errorf("not a @test attribute: @%s", name) 204 203 } 205 204 206 - parsed := internal.ParseAttrBody(a.Pos(), body) 207 - if parsed.Err != nil { 208 - return parsedTestAttr{}, parsed.Err 205 + attr := internal.ParseAttr(astAttr) 206 + if attr.Err != nil { 207 + return parsedTestAttr{}, attr.Err 209 208 } 210 209 211 210 result := parsedTestAttr{ 212 - raw: parsed, 213 - srcAttr: a, 211 + raw: attr, 212 + srcAttr: astAttr, 214 213 } 215 214 216 - if len(parsed.Fields) == 0 || (len(parsed.Fields) == 1 && parsed.Fields[0] == internal.KeyValue{}) { 215 + if len(attr.Fields) == 0 || (len(attr.Fields) == 1 && attr.Fields[0] == internal.KeyValue{}) { 217 216 // @test() — empty placeholder or bare marker. 218 217 result.directive = "" 219 218 return result, nil ··· 222 221 // The first field determines the directive. 223 222 // Case 1: key=value form like desc="hello", shareID=name — directive is the key. 224 223 // Case 2: positional form like eq, err, kind — directive (with optional :vN suffix) is the value. 225 - f0 := parsed.Fields[0] 224 + f0 := attr.Fields[0] 226 225 if f0.Key() != "" { 227 226 dir := f0.Key() 228 227 // Key-based directives may carry a version suffix: "shareID:v3" → directive="shareID", version="v3". ··· 246 245 // Parse directive-specific sub-options. 247 246 switch result.directive { 248 247 case "err": 249 - ea, err := parseErrArgs(parsed) 248 + ea, err := parseErrArgs(attr) 250 249 if err != nil { 251 250 return result, err 252 251 } ··· 257 256 // Positional args (kv.Key() == "") are accepted by directives as needed. 258 257 // Directives with their own flag parsers (err, todo, skip, shareID) are 259 258 // responsible for validating their own flags. 260 - for _, kv := range parsed.Fields[1:] { 259 + for _, kv := range attr.Fields[1:] { 261 260 switch kv.Key() { 262 261 case "hint": 263 262 result.hint = kv.Value() ··· 1449 1448 // attrHasSkip reports whether the raw attribute body contains a skip:<ver> arg 1450 1449 // at position 2 or later. Returns the version string (e.g. "v3") and true 1451 1450 // when a skip arg is found; returns "", false otherwise. 1452 - func attrHasSkip(raw internal.Attr) (ver string, ok bool) { 1451 + func attrHasSkip(raw *internal.Attr) (ver string, ok bool) { 1453 1452 for i := 2; i < len(raw.Fields); i++ { 1454 1453 text := raw.Fields[i].Text() 1455 1454 if text == "skip" {
+5 -2
internal/cuetxtar/inline_err.go
··· 30 30 "golang.org/x/tools/txtar" 31 31 32 32 "cuelang.org/go/cue" 33 + "cuelang.org/go/cue/ast" 33 34 cueerrors "cuelang.org/go/cue/errors" 34 35 "cuelang.org/go/cue/token" 35 36 "cuelang.org/go/internal" ··· 111 112 112 113 // parseErrArgs extracts err sub-options from an already-parsed Attr. 113 114 // The attribute body is expected to start with "err" as the first positional arg. 114 - func parseErrArgs(a internal.Attr) (errArgs, error) { 115 + func parseErrArgs(a *internal.Attr) (errArgs, error) { 115 116 var ea errArgs 116 117 // Start from index 1 (index 0 is "err"). 117 118 for _, kv := range a.Fields[1:] { ··· 142 143 return ea, fmt.Errorf("@test(err, suberr=...): %w", err) 143 144 } 144 145 // Reuse parseErrArgs by building a synthetic "err, <inner>" attr body. 145 - syntheticAttr := internal.ParseAttrBody(token.NoPos, "err, "+inner) 146 + syntheticAttr := internal.ParseAttr(&ast.Attribute{ 147 + Text: fmt.Sprintf("@test(err, %s)", inner), 148 + }) 146 149 subEA, err := parseErrArgs(syntheticAttr) 147 150 if err != nil { 148 151 return ea, fmt.Errorf("@test(err, suberr=...): %w", err)
+2 -3
internal/encoding/yaml/encode.go
··· 239 239 // Returns an error if the attribute is malformed. 240 240 func extractYAMLTag(attrs []*ast.Attribute) (string, error) { 241 241 for _, attr := range attrs { 242 - key, body := attr.Split() 243 - if key != "yaml" { 242 + if attr.Name() != "yaml" { 244 243 continue 245 244 } 246 - parsed := internal.ParseAttrBody(attr.Pos(), body) 245 + parsed := internal.ParseAttr(attr) 247 246 if parsed.Err != nil { 248 247 return "", parsed.Err 249 248 }