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

Fixed UDT syntax highlighting #14372

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Fixed UDT syntax highlighting #14372

merged 2 commits into from
Jun 20, 2024

Conversation

majastrz
Copy link
Member

@majastrz majastrz commented Jun 19, 2024

Fixed the recent regression in syntax highlighting for user defined types:

  • Now emitting SemanticTokenType.Namespace for namespace symbols.
  • Now emitting SemanticTokenType.Type for TypeVariableAccessSyntax that resolves to a valid type symbol.
  • Now emitting SemanticTokenType.Keyword for null, true and false type keywords.

Examples of changes:
image

I will look into adding tests separately since we don't currently have baselines or meaningful unit tests for syntax highlighting. (I recall we had some issues with that in the past, but will need to confirm.)

This fixes #14147

Microsoft Reviewers: Open in CodeFlow

Copy link
Contributor

github-actions bot commented Jun 19, 2024

Test this change out locally with the following install scripts (Action run 9590262619)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 9590262619
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 9590262619"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 9590262619
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 9590262619"

Copy link
Contributor

@jeskew jeskew left a comment

Choose a reason for hiding this comment

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

Nice! :shipit:

Copy link
Contributor

Dotnet Test Results

    72 files   -     36      72 suites   - 36   23m 38s ⏱️ - 8m 42s
10 891 tests  -     20  10 890 ✅  -     20  1 💤 ±0  0 ❌ ±0 
25 678 runs   - 12 835  25 676 ✅  - 12 834  2 💤  - 1  0 ❌ ±0 

Results for commit 28c3da6. ± Comparison against base commit bf8a344.

private static SemanticTokenType GetSemanticTokenForPotentialTypeSymbol(Symbol? symbol) =>
symbol switch
{
PropertySymbol property => GetSemanticTokenForPotentialTypeSymbol(property.Type),
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this end up highlighting an imported variable (accessed as a property on a wildcard import) as a type? E.g.,

mod.bicep

@export()
var foo = 'foo'

main.bicep

import * as mod from 'mod.bicep'

output foo string = mod.foo

Copy link
Member Author

Choose a reason for hiding this comment

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

It highlights the mod part as a namespace but foo is highlighted as a variable:
image

@majastrz majastrz merged commit e307d17 into main Jun 20, 2024
44 checks passed
@majastrz majastrz deleted the majastrz/type-highlighting branch June 20, 2024 01:37
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.

Types are not highlighted in a different color
2 participants