Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Introduce PathContext and DecoderContext #92

Merged
merged 5 commits into from
Oct 29, 2021
Merged

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Oct 21, 2021

Summary

As it is usually the case with refactoring PRs, the diff is not small. However I tried to separate all changes into some logical steps per commits in the log.

The overall intention is to bring the main Decoder API closer to LSP methods and move some lower-level APIs into a new reference package. This then allows us to reduce the footprint of handlers in LS.

All these changes allow us to reuse a single instance of the decoder in LS, as demonstrated in hashicorp/terraform-ls#689 which aids readability.

Motivation

All this is mainly to enable future Terraform LS module-related work as described in hashicorp/vscode-terraform#715 Most of this work can now be done within this library, as opposed to LS itself, such that future other language servers can benefit from it.

New Concept of a Path

A concept of "path" is introduced, where path represents a path with parsed files of a particular single language ID (such as *.tf in case of Terraform). Relatedly the "context" required for the decoder is now exposed more systematically via PathContext (for path-dependent context) and DecoderContext for global/shared settings. This provides more flexibility and cleaner API than a setter for each option.

Generalized Code Lens

It also introduces a more generalized concept of code lens, as represented by this interface:

type CodeLensFunc func(ctx context.Context, path Path, file string) ([]CodeLens, error)

type CodeLens struct {
	Range   hcl.Range
	Command Command
}

type Command struct {
	Title     string
	ID        string
	Arguments []CommandArgument
}

type CommandArgument interface {
	MarshalJSON() ([]byte, error)
}

In theory we could introduce some kind of stdlib of code lenses which are useful for any HCL2 config, but the trouble is that the "command" interface is still very loosely defined undefined by LSP itself, there's very little convention and so it's unclear how useful would such stdlib be at this point.

However this can pave a way for a similar generalized API for code actions in the future.


Snapshots of the generated docs probably best illustrate the difference.

Before
https://pkg.go.dev/github.com/hashicorp/hcl-lang@v0.0.0-20211021151402-2f992812c925/decoder#pkg-index

type Decoder

  • func (d *Decoder) CandidatesAtPos(filename string, pos hcl.Pos) (lang.Candidates, error)
  • func (d *Decoder) CollectReferenceOrigins() (lang.ReferenceOrigins, error)
  • func (d *Decoder) CollectReferenceTargets() (lang.ReferenceTargets, error)
  • func (d *Decoder) Filenames() []string
  • func (d *Decoder) HoverAtPos(filename string, pos hcl.Pos) (*lang.HoverData, error)
  • func (d *Decoder) InnermostReferenceTargetsAtPos(file string, pos hcl.Pos) (lang.ReferenceTargets, error)
  • func (d *Decoder) LinksInFile(filename string) ([]lang.Link, error)
  • func (d *Decoder) LoadFile(filename string, f *hcl.File) error
  • func (d *Decoder) OutermostReferenceTargetsAtPos(file string, pos hcl.Pos) (lang.ReferenceTargets, error)
  • func (d *Decoder) ReferenceOriginAtPos(filename string, pos hcl.Pos) (*lang.ReferenceOrigin, error)
  • func (d *Decoder) ReferenceOriginsTargeting(refTarget lang.ReferenceTarget) (lang.ReferenceOrigins, error)
  • func (d *Decoder) ReferenceTargetForOrigin(refOrigin lang.ReferenceOrigin) (*lang.ReferenceTarget, error)
  • func (d *Decoder) ReferenceTargetsInFile(file string) (lang.ReferenceTargets, error)
  • func (d *Decoder) SemanticTokensInFile(filename string) ([]lang.SemanticToken, error)
  • func (d *Decoder) SetReferenceOriginReader(f ReferenceOriginReader)
  • func (d *Decoder) SetReferenceTargetReader(f ReferenceTargetReader)
  • func (d *Decoder) SetSchema(schema *schema.BodySchema)
  • func (d *Decoder) SetUtmMedium(medium string)
  • func (d *Decoder) SetUtmSource(src string)
  • func (d *Decoder) Symbols(query string) ([]Symbol, error)
  • func (d *Decoder) SymbolsInFile(filename string) ([]Symbol, error)
  • func (d *Decoder) UseUtmContent(use bool)

After
https://pkg.go.dev/github.com/hashicorp/hcl-lang@v0.0.0-20211029104910-0a2d253a116f/decoder
https://pkg.go.dev/github.com/hashicorp/hcl-lang@v0.0.0-20211029104910-0a2d253a116f/reference

type Decoder

  • func (d *Decoder) Path(path lang.Path) (*PathDecoder, error)
  • func (d *Decoder) SetContext(ctx DecoderContext)
  • func (d *Decoder) Symbols(ctx context.Context, query string) ([]Symbol, error)

type PathDecoder

  • func (d *PathDecoder) CandidatesAtPos(filename string, pos hcl.Pos) (lang.Candidates, error)
  • func (d *PathDecoder) CodeLensesForFile(ctx context.Context, file string) ([]lang.CodeLens, error)
  • func (d *PathDecoder) CollectReferenceOrigins() (reference.Origins, error)
  • func (d *PathDecoder) CollectReferenceTargets() (reference.Targets, error)
  • func (d *PathDecoder) HoverAtPos(filename string, pos hcl.Pos) (*lang.HoverData, error)
  • func (d *PathDecoder) LinksInFile(filename string) ([]lang.Link, error)
  • func (d *PathDecoder) ReferenceOriginsTargetingPos(file string, pos hcl.Pos) ReferenceOrigins
  • func (d *PathDecoder) ReferenceTargetForOriginAtPos(file string, pos hcl.Pos) (*ReferenceTarget, error)
  • func (d *PathDecoder) SemanticTokensInFile(filename string) ([]lang.SemanticToken, error)
  • func (d *PathDecoder) SymbolsInFile(filename string) ([]Symbol, error)

Notably the new reference-related methods expose more minimal structs representing just necessary details about references:

which make them far more suitable to also represent cross-path references.

@radeksimko radeksimko changed the title refactor: Introduce DirContext and DecoderContext refactor: Introduce PathContext and DecoderContext Oct 21, 2021
@radeksimko radeksimko force-pushed the f-dir-context branch 13 times, most recently from f66b386 to b5fa835 Compare October 28, 2021 10:52
@radeksimko radeksimko force-pushed the f-dir-context branch 2 times, most recently from 7be6d0a to 0a2d253 Compare October 29, 2021 10:49
@radeksimko radeksimko marked this pull request as ready for review October 29, 2021 11:10
@radeksimko radeksimko requested a review from a team October 29, 2021 11:35
Copy link
Contributor

@jpogran jpogran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After Radek walked me through the changes, and explained the refactoring, this looks like it will be a great change to make. I think this will not only help for modules, but also for the Prefill Required Fields CodeAction I want to implement later.

@radeksimko radeksimko merged commit 70678d4 into main Oct 29, 2021
@radeksimko radeksimko deleted the f-dir-context branch October 29, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants