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

First segment of references and object contents mis-highlighted #79

Open
scott-doyland-burrows opened this issue Jun 21, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@scott-doyland-burrows
Copy link

Extension Version

v2.26.20230511

VS Code Version

Version: 1.79.1 (user setup)
Commit: 4cb974a7aed77a74c7813bdccd99ee0d04901215
Date: 2023-06-12T16:14:05.102Z
Electron: 22.5.7
Chromium: 108.0.5359.215
Node.js: 16.17.1
V8: 10.8.168.25-electron.0
OS: Windows_NT x64 10.0.19044

Operating System

Windows 10

Steps to Reproduce

Setup files as below - note the colours in the main.pkr.hcl file for:

    repo          = var.GITHUB_REPOSITORY
    branch        = var.REF_NAME
    event_type    = var.EVENT_NAME

Expected Behavior

Colours should be correct.

Actual Behavior

Colours incorrect on lines 12/13/14.

image

Configuration

file.pkr.hcl

source "amazon-ebs" "nat-ubuntu" {
  ami_name      = "digital-images-nat-ubuntu-${local.timestamp}"
  source_ami    = data.amazon-ami.nat-ubuntu.id
  instance_type = "t4g.micro"
  region        = "eu-west-1"
  ssh_username  = "ubuntu"
  ami_regions   = var.ami_regions

  tags = {
    base_ami_id   = "{{ .SourceAMI }}"
    base_ami_name = "{{ .SourceAMIName }}"
    repo          = var.GITHUB_REPOSITORY
    branch        = var.REF_NAME
    event_type    = var.EVENT_NAME
    production    = var.REF_NAME == "main" ? "true" : "false"
  }

  vpc_filter {
    filters = {
      "isDefault" : "false"
    }
  }
  subnet_filter {
    filters = {
      "tag:Name" : "primary*"
    }
    random = true
  }

  ami_users = var.accounts
}

variable.pkr.hcl

variable "accounts" {
  type = list(string)

  default = []
}

variable "ami_regions" {
  type = list(string)

  default = [
    "us-east-1",
    "eu-central-1",
    "eu-west-2",
    "sa-east-1",
  ]
}

variable "GITHUB_REPOSITORY" {
  description = "GitHub repo"

  type = string

  default = "Hogarth-Worldwide/terraform-aws-amis-digital"
}

variable "REF_NAME" {
  description = "GitHub branch"

  type = string

  default = "local_running"
}

variable "EVENT_NAME" {
  description = "GitHub event name"

  type = string

  default = "local_running"
}
@radeksimko
Copy link
Member

Hi @scott-doyland-burrows
Based on the screenshot I'm guessing there are two problems.

With default VS Code configuration, the same snippet of code gets highlighted a bit differently to me:
Screenshot 2023-06-21 at 09 32 48
(note the difference in the first segments of addresses where here none of them are highlighted)
This would suggest that you may have configured VS Code such that it treats *.hcl or *.pkr.hcl as terraform language, making the Terraform extension to highlight it. The Terraform extension is designed to only work with Terraform code (i.e. *.tf and *.tfvars), not arbitrary HCL based language. Also if it's configured that way it will likely lead to other kind of problems when attempting to format the code and do things other than just highlighting as all of the functionality is - again - tailored to Terraform.

That said even in my screenshot it looks like something is still off in that the first address segment is highlighted differently when the address is inside a nested block (tags in this case), unless it's part of a conditional expression. That is most certainly a bug we'll need to address.

Thank you for the report.

@radeksimko radeksimko changed the title pkr.hcl files do not highlight correctly First segment of reference mis-highlighted when inside nested block Jun 21, 2023
@radeksimko radeksimko added the bug Something isn't working label Jun 21, 2023
@scott-doyland-burrows
Copy link
Author

You are correct, it was set to use the terraform extension. I was actually flicking between the two extensions to see the difference and mistakenly submitted the screenshot when the terraform extension was in use.

But as you say, something does seem incorrect with the HCL extension - and I see the same as you when I use the correct extension.

image

In your screenshot, the source_ami line shows data in white, is that correct?

@radeksimko radeksimko changed the title First segment of reference mis-highlighted when inside nested block First segment of reference mis-highlighted Jun 21, 2023
@radeksimko
Copy link
Member

In your screenshot, the source_ami line shows data in white, is that correct?

Good catch, yes, that could be considered a bug too, which applies to all references - we can look into both at the same time - i.e. no need to file a separate issue for that.

@radeksimko
Copy link
Member

I recall some semi-related earlier conversations we had when implementing the highlighting grammar about how this works out together with semantic highlighting which - one may expect - highlights valid references differently to invalid ones (i.e. such that refer to block/attribute that doesn't exist). That is something we need to dig through as part of hashicorp/terraform-ls#1304 but it's mostly affecting Terraform rather than plain HCL. It just happens that we use bulk of the grammar for both HCL and Terraform.

I'm not sure if that issue will impact how we (can) fix this, I'm merely sharing some context on what's likely behind the bug.

@radeksimko radeksimko transferred this issue from hashicorp/vscode-hcl Jun 23, 2023
@radeksimko
Copy link
Member

I transferred the issue to the repository where we host the grammars which will need fixing.

I also did some investigation only to discover that this is a slightly wider problem of how we treat references in general, which may also have something to do with the limitations of TextMate grammars and regex.

syntax/src/_main.yml

Lines 374 to 389 in 13b5b4f

attribute_access:
begin: \.(?!\*)
beginCaptures:
"0":
name: keyword.operator.accessor.hcl
end: '[[:alpha:]][\w-]*|\d*'
comment: Matches traversal attribute access such as .attr
endCaptures:
"0":
patterns:
- match: (?!null|false|true)[[:alpha:]][\w-]*
comment: Attribute name
name: variable.other.member.hcl
- match: \d+
comment: Optional attribute index
name: constant.numeric.integer.hcl

^ that is the part of the grammar responsible for detecting references

There's a couple of problems with it:

  1. it only detects reference steps on the 2nd and further positions, never the first one
  2. even though Terraform specifically probably doesn't have such a reference it doesn't detect single-step (practically any identifier which isn't boolean, null, or type declaration like number)

I'm not sure we can fix (2) reliably without going through a lot of pain in crafting complex regular expressions that exclude all the false positives but there should be some way of fixing (1).

I tried changing the expression in a few ways but I always ran into false positive matches because once the leading . becomes optional, a lot of things suddenly match the pattern.

Then there's also something wrong with the way we detect objects, or rather the expressions inside of it (tags = { ... }) which affects references in general and operators like == too.

syntax/src/_main.yml

Lines 248 to 299 in 13b5b4f

objects:
name: meta.braces.hcl
begin: \{
beginCaptures:
"0":
name: punctuation.section.braces.begin.hcl
end: \}
endCaptures:
"0":
name: punctuation.section.braces.end.hcl
patterns:
- include: "#comments"
- include: "#objects"
- include: "#inline_for_expression"
- include: "#inline_if_expression"
- match: \b((?!null|false|true)[[:alpha:]][[:alnum:]_-]*)\s*(\=\>?)\s*
comment: Literal, named object key
captures:
"1":
name: meta.mapping.key.hcl variable.other.readwrite.hcl
"2":
name: keyword.operator.assignment.hcl
patterns:
- match: \=\>
name: storage.type.function.hcl
- match: \b((").*("))\s*(\=)\s*
comment: String object key
captures:
"1":
name: meta.mapping.key.hcl string.quoted.double.hcl
"2":
name: punctuation.definition.string.begin.hcl
"3":
name: punctuation.definition.string.end.hcl
"4":
name: keyword.operator.hcl
- begin: ^\s*\(
comment: Computed object key (any expression between parens)
name: meta.mapping.key.hcl
beginCaptures:
"0":
name: punctuation.section.parens.begin.hcl
end: (\))\s*(=|:)\s*
endCaptures:
"1":
name: punctuation.section.parens.end.hcl
"2":
name: keyword.operator.hcl
patterns:
- include: "#attribute_access"
- include: "#attribute_splat"
- include: "#object_key_values"

^ that is the snippet responsible for detecting object expressions, but I did not dig too deep into it to understand what exactly is wrong with it


To summarize, from UX perspective there are three visible issues. I used 2 different themes to visualise it below:
Screenshot 2023-06-23 at 11 33 38
Screenshot 2023-06-23 at 11 33 57

@radeksimko radeksimko changed the title First segment of reference mis-highlighted First segment of references and object contents mis-highlighted Jun 23, 2023
@dbanck
Copy link
Member

dbanck commented Mar 20, 2024

I fixed issue 3 in #112.

1 & 2 still remain as they are a bit more involved as Radek explained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants