parser.y: Always populate SelectField offset

[kyleconroy]

👋 This is my time contributing code to this project, so please let me know if I need to do more work to get this approved. I'm happy to add tests, documentation, etc.

What problem does this PR solve?

This is a partial solution to #910

What is changed and how it works?

ast.SelectField already has an Offset field. I've updated parser.y to populate this field whenever an ast.SelectField is created.

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #943 into master will decrease coverage by 0.06%.
The diff coverage is 62.50%.

@@            Coverage Diff             @@
##           master     #943      +/-   ##
==========================================
- Coverage   78.46%   78.40%   -0.07%     
==========================================
  Files          40       40              
  Lines       14916    15013      +97     
==========================================
+ Hits        11704    11771      +67     
- Misses       2521     2544      +23     
- Partials      691      698       +7     

[kyleconroy]

Any feedback for this pull request? Is this something you'd be interested in merging if I fixed the test failures?

[tangenta]

The offset field has been initialized here:

parser/parser.y

Lines 4845 to 4851 in 12de604

	FieldList:
	Field
	{
	field := $1.(*ast.SelectField)
	field.Offset = parser.startOffset(&yyS[yypt])
	$$ = []*ast.SelectField{field}
	}

What's the purpose to make this change? Are there any tests or use cases?

[kyleconroy]

Hmm, if this is being populated I'm not sure why I needed to change parser.y at all. I'll need to investigate why it wasn't working for me.

[tangenta]

Any feedback for this pull request? Is this something you'd be interested in merging if I fixed the test failures?

Sorry for the late reply. I thought this is still WIP..

[kyleconroy]

Sorry for the late reply. I thought this is still WIP..

No worries! I was hoping to get feedback before I did too much work. Thanks for explaining how Offset is currently being populated, as I think it makes this PR unnecessary.

[kyleconroy]

I finally got around to testing this; the current behavior works perfectly.