search

dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.84k stars 777 forks source link

QuoteIdentifierIfNeeded with single underscore #7681

Closed nojaf closed 2 years ago

nojaf commented 4 years ago

Repro steps

FSharp.Compiler.SourceCodeServices.PrettyNaming.QuoteIdentifierIfNeeded "_"

Expected behavior

I was expecting the function to return _

Actual behavior

``_``

Is this correct?

Related information

Bug found in Fantomas: https://github.com/fsprojects/fantomas/issues/493

Provide any related information (optional):

abelbraaksma commented 4 years ago

The identifier "_" is not a valid identifier. It is a valid pattern. This allows it to be used in let bindings, but not in places (like member names, function names) where a so-called ident is expected.

If you wanted to use the underscore as valid identifier, it needs to be back-quoted. Hence I think this output is correct.

Don't confuse it with the recently newly allowed _ for the self identifier in instance member declarations. That is still not a valid ident, it's a special value that says "I don't need the self reference in the body of my member".

cartermp commented 4 years ago

Given this history of this line: https://github.com/dotnet/fsharp/blob/6a648d8bceb011000b1a7c7151af227b1347c5c8/src/fsharp/lexhelp.fs#L280

The answer is probably, "yes" it should be expected. However, it might be good to special cause discards for member declarations in tools.

NinoFloris commented 4 years ago

I wanted to start using _ for members but with ionide now having fantomas support I'm not very keen on backticks everywhere. How should we best fix this?

abelbraaksma commented 4 years ago

Is this a bug with Fantomas, Ionide or F#? I'm confused.

Self identifiers in F# source code oughtn't be backtick quoted. Without backticks _ is ignored, with them it's a usable self identifier.

auduchinok commented 4 years ago

@abelbraaksma It's a limitation in the current FCS API. _ is not a valid identifier, so it actually works as designed. FCS users want to use this API to check if something on an identifier place is allowed as is and such a check should probably be a separate API.

nojaf commented 2 years ago

This appears to be fixed now:

FSharp.Compiler.Syntax.PrettyNaming.AddBackticksToIdentifierIfNeeded "_"
|> printfn "FSharp.Compiler.Syntax.PrettyNaming.AddBackticksToIdentifierIfNeeded :  %s"

yields

FSharp.Compiler.Syntax.PrettyNaming.AddBackticksToIdentifierIfNeeded :  _

any idea what changed here?