Refactor struct field selection to Inspector helper #49
Conversation
| // The 0th node in the stack is the *ast.File. | ||
| // The 1st node in the stack is the *ast.GenDecl. |
There was a problem hiding this comment.
// The 0th node in the stack is the *ast.File.
// The 1st node in the stack is the *ast.GenDecl.
Is this description is necessary ? If so, I think it requires about stack[len(stack)-3]
There was a problem hiding this comment.
I've got a comment within the if statement, line 61 and 62 that explains what that particular one is about, do you think I need more than that?
|
Thank you for great improvement 🙇 |
JoelSpeed
force-pushed
the
inspector-helper
branch
from
0fb4755 to
7ea8c53
Compare
|
As far as I'm concerned, I can confirm the PR indeed fixes the bug I was experiencing in #48. |
| } | ||
| } | ||
|
|
||
| type Interface interface { |
There was a problem hiding this comment.
Do we also need to test a type that uses composition of the interface like:
type ComposedThing struct {
Interface
Foo string
}?
There was a problem hiding this comment.
I don't think you can embed interfaces like that, you can embed other structs, not interfaces, I thought we had that case already though
Apparently you can
There was a problem hiding this comment.
This would just mean you have to initialise the interface, inside the struct, with something that implements the interface.
At the moment I can't see why anyone would be using that inside a Kube API type, but maybe there are other types in the same package that this might effect?
For now I'm tempted to leave this out until we see a use case
| // Inspector is an interface that allows for the inspection of fields in structs. | ||
| type Inspector interface { | ||
| // InspectFields is a function that iterates over fields in structs. | ||
| InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers)) |
There was a problem hiding this comment.
Nit: Make the input func a specific type?
There was a problem hiding this comment.
I had that, and moved away from it as my code completion was being annoying and not understanding how to populate... Maybe will move back in the future
There was a problem hiding this comment.
Someone's LSP needs an update ;)
This expects the recent changes to the struct field extraction out into a helper, so that we can centralise things like ignoring structs that are declared within functions, or ignored json tags
"-". This also now makes sure that the linters affected by this change do not inspect interface declarations which was the cause for #48.Fixes #48
CC @mandre @sivchari