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

Implement basic nested language detection support #1159

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Sep 4, 2023

WHAT

🤖 Generated by Copilot at ddd5195

This pull request adds a new feature to detect and report nested languages in F# files, such as XML or SQL, using a compiler analyzer and a custom LSP notification. It modifies the FsAutoComplete.Core and FsAutoComplete.Lsp projects to implement the feature and the FsAutoComplete.Tests.Lsp project to test it. It also fixes some indentation and style issues in various files and adds a new configuration for running the tests in VS Code.

🤖 Generated by Copilot at ddd5195

We're sailing on the F# sea, with nested languages to see
We've got a new notification, to send to the LSP station
So heave away, me hearties, heave away
We'll make the code more tidy, with SyntaxCollectorBase and findNestedLanguages

🌐🛠️🧪

WHY

Part of ionide/ionide-vscode-fsharp#1912, blocked by dotnet/fsharp#15925.

HOW

🤖 Generated by Copilot at ddd5195

  • Add nested language feature to detect and report embedded languages in F# files (link, link, link, link, link, link, link, link)
  • Extend SyntaxCollectorBase type with AbstractClass attribute and default methods for convenience and clarity (link, link)
  • Enable preview language features in FsAutoComplete.Core.fsproj to use nameof and open type syntax (link)
  • Add configuration for running nested language tests with Roslyn source text in launch.json (link)
  • Add unit tests for nested language feature in NestedLanguageTests.fs and include them in generalTests in Program.fs (link, link)
  • Fix indentation and formatting issues in Program.fs and Server.fs according to F# style guide (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Fix indentation of record fields and member definitions in Server.fsi according to F# style guide (link, link)
  • Remove unused open statement in FSharpLspClient.fs (link)
  • Comment out findNestedLanguages function as a potential configuration flag in AdaptiveFSharpLspServer.fs (link)

@@ -12,6 +11,14 @@
open System.Threading
open IcedTasks

type NestedLanguage =
{ Language: string
Ranges: Types.Range[] }

Check notice

Code scanning / Ionide.Analyzers.Cli

Detect if type[] is used instead of type array Note

Prefer postfix syntax for arrays.

type TextDocumentNestedLanguages =
{ TextDocument: VersionedTextDocumentIdentifier
NestedLanguages: NestedLanguage[] }

Check notice

Code scanning / Ionide.Analyzers.Cli

Detect if type[] is used instead of type array Note

Prefer postfix syntax for arrays.
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
src/FsAutoComplete.Core/NestedLanguages.fs Fixed Show fixed Hide fixed
@baronfel
Copy link
Contributor Author

BIG update here - this works off of StringSyntax attributes for F# functions, but not for F#-defined class members or BCL-provided class members. There's a gap in the FCS APIs for providing Attributes for these members. I've raised it upstream. In the meantime, this is enough to get going on the Ionide-side for implementation!

src/FsAutoComplete.Core/NestedLanguages.fs Dismissed Show dismissed Hide dismissed
Comment on lines +28 to +104
let private discoverRangesToRemoveForInterpolatedString
(stringKind: SynStringKind)
(parts: SynInterpolatedStringPart[])
=
parts
|> Array.indexed
|> Array.collect (fun (index, part) ->
match part with
| SynInterpolatedStringPart.FillExpr(fillExpr = e) -> [| e.Range |]
// for the first part we have whatever 'leading' element on the left and a trailing interpolation piece (which can include a format specifier) on the right
| SynInterpolatedStringPart.String(range = range) when index = 0 ->
[|
// leading tokens adjustment
// GAP: we don't know how many interpolation $ or " there are, so we are guessing
match stringKind with
| SynStringKind.Regular ->
// 'regular' means $" leading identifier
range.WithEnd(range.Start.WithColumn(range.StartColumn + 2))
| SynStringKind.TripleQuote ->
// 'triple quote' means $""" leading identifier
range.WithEnd(range.Start.WithColumn(range.StartColumn + 4))
// there's no such thing as a verbatim interpolated string
| SynStringKind.Verbatim -> ()

// trailing token adjustment- only an opening bracket {
// GAP: this is the feature gap - we don't know about format specifiers
range.WithStart(range.End.WithColumn(range.EndColumn - 1))

|]
// for the last part we have a single-character interpolation bracket on the left and the 'trailing' string elements on the right
| SynInterpolatedStringPart.String(range = range) when index = parts.Length - 1 ->
[|
// leading token adjustment - only a closing bracket }
range.WithEnd(range.Start.WithColumn(range.StartColumn + 1))

// trailing tokens adjustment
// GAP: we don't know how many """ to adjust for triple-quote interpolated string endings
match stringKind with
| SynStringKind.Regular ->
// 'regular' means trailing identifier "
range.WithStart(range.End.WithColumn(range.EndColumn - 1))
| SynStringKind.TripleQuote ->
// 'triple quote' means trailing identifier """
range.WithStart(range.End.WithColumn(range.EndColumn - 3))
// no such thing as verbatim interpolated strings
| SynStringKind.Verbatim -> () |]
// for all other parts we have a single-character interpolation bracket on the left and a trailing interpolation piece (which can include a format specifier) on the right
| SynInterpolatedStringPart.String(range = range) ->
[|
// leading token adjustment - only a closing bracket }
range.WithEnd(range.Start.WithColumn(range.StartColumn + 1))
// trailing token adjustment- only an opening bracket {
// GAP: this is the feature gap - we don't know about format specifiers here
range.WithStart(range.End.WithColumn(range.EndColumn - 1)) |])

let private (|Ident|_|) (e: SynExpr) =
match e with
| SynExpr.Ident(ident) -> Some([ ident ])
| SynExpr.LongIdent(longDotId = SynLongIdent(id = ident)) -> Some ident
| _ -> None

/// in order for nested documents to be recognized as their document types, the string quotes (and other tokens) need to be removed
/// from the actual string content.
let private removeStringTokensFromStringRange (kind: SynStringKind) (range: Range) : Range array =
match kind with
| SynStringKind.Regular ->
// we need to trim the double-quote off of the start and end
[| Range.mkRange range.FileName range.Start (range.Start.WithColumn(range.StartColumn + 1))
Range.mkRange range.FileName (range.End.WithColumn(range.EndColumn - 1)) range.End |]
| SynStringKind.Verbatim ->
// we need to trim the @+double-quote off of the start and double-quote off the end
[| Range.mkRange range.FileName range.Start (range.Start.WithColumn(range.StartColumn + 2))
Range.mkRange range.FileName (range.End.WithColumn(range.EndColumn - 1)) range.End |]
| SynStringKind.TripleQuote ->
// we need to trim the @+double-quote off of the start and double-quote off the end
[| Range.mkRange range.FileName range.Start (range.Start.WithColumn(range.StartColumn + 2))
Range.mkRange range.FileName (range.End.WithColumn(range.EndColumn - 1)) range.End |]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nojaf @vzarytovskii look at all of the nonsense string math I have to do in these two adjustment functions to get the 'real' content of the string :( Do you think the AST could/should learn any trivia here to make this kind of job easier? There are a few different pieces that are troublesome:

  • string/interpolated string start - this can be ", or @", or """, or now with interpolated strings any number of $ followed by """. this is untenable to get right currently.
  • string/interpolated string end - this can be ", """, or arbitrary " now. similarly bad :(
  • format specifiers - these are relevant to formatting, but are not part of the logical string content. currently they are part of the reported string range and this means to do it 'right' I'd need to get the full string's content and do a subsequent parse
  • string interpolation close/open brackets - these can be any number of {/} now with the expanded interpolation RFC

Essentially I'd love some kind of synthesized range for the logical content of a string in the AST, because all of this math is very error prong.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Fantomas, we currently rely on the ranges of SynInterpolatedStringParts to restore the string. So, as long as these are accurate, we can grab what we need from ISourceText and no longer deal with this. In your case, it is indeed very messy to get right.

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