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

Strict mode strips literal strings #560

Closed
Smaug123 opened this issue Nov 18, 2019 · 6 comments · Fixed by #2023
Closed

Strict mode strips literal strings #560

Smaug123 opened this issue Nov 18, 2019 · 6 comments · Fixed by #2023

Comments

@Smaug123
Copy link
Contributor

Issue created from fantomas-ui

Strict mode causes the @ to be stripped from strings, possibly causing code to stop compiling.

Code

let s = @"\"

Error

let s = "\"

Options

Fantomas Next - 3.0.1-11/13/2019

Name Value
IndentOnTryWith false
IndentSpaceNum 4
KeepNewlineAfter false
PageWidth 120
ReorderOpenDeclaration false
SemicolonAtEndOfLine false
SpaceAfterComma false
SpaceAfterSemicolon false
SpaceAroundDelimiter false
SpaceBeforeArgument false
SpaceBeforeColon false
StrictMode true
@nojaf
Copy link
Contributor

nojaf commented Nov 18, 2019

Hello @Smaug123 ,

Strict mode means it will only use the information found in the UnTyped AST to print out the formatted code.

The AST, in this case, looks like this:

ImplFile
  (ParsedImplFileInput
     ("Script.fsx",true,QualifiedNameOfFile Script$fsx,[],[],
      [SynModuleOrNamespace
         ([Script],false,AnonModule,
          [Let
             (false,
              [Binding
                 (None,NormalBinding,false,false,[],
                  PreXmlDoc ((1,5),FSharp.Compiler.Ast+XmlDocCollector),
                  SynValData
                    (None,SynValInfo ([],SynArgInfo ([],false,None)),None),
                  Named
                    (Wild Script.fsx (1,4--1,5) IsSynthetic=false,s,false,None,
                     Script.fsx (1,4--1,5) IsSynthetic=false),None,
                  Const
                    (String ("\",Script.fsx (1,8--1,12) IsSynthetic=false),
                     Script.fsx (1,8--1,12) IsSynthetic=false),
                  Script.fsx (1,4--1,5) IsSynthetic=false,
                  SequencePointAtBinding
                    Script.fsx (1,0--1,12) IsSynthetic=false)],
              Script.fsx (1,0--1,12) IsSynthetic=false)],PreXmlDocEmpty,[],None,
          Script.fsx (1,0--1,12) IsSynthetic=false)],(true, true)))

The @ sign is not part of Const(String ("\",Script.fsx (1,8--1,12) IsSynthetic=false), so it is inpossible to know without looking at the original source code.

This is a known limitation and your best bet to resolve this is to open an issue at the dotnet/fsharp repo.

@Smaug123
Copy link
Contributor Author

I'm not convinced that Fantomas should just pass this off to the F# compiler. Isn't the "\" that you quoted just an unescaped string? It's possible to format an unescaped string canonically, by printing it with an @ in front of it and duplicating any double-quotes inside it, if I understand correctly.

@nojaf
Copy link
Contributor

nojaf commented Oct 2, 2020

@Smaug123 as we currently don't have this issue in a non-strict mode, I made a suggestion to store this information in the Syntax tree compiler dotnet/fsharp#10209.

Yes, Fantomas could try and detect this based on the content of the string but the compiler at some point has this information, so it seems like a good case to get my feet wet in extended the compiler itself.

@Smaug123
Copy link
Contributor Author

Smaug123 commented Oct 2, 2020

Good idea - this is certainly not a high-priority ticket, we don't expect to be using strict mode.

@nojaf
Copy link
Contributor

nojaf commented Dec 21, 2020

Following up on this, with a lot of help of Chet, we were able to submit dotnet/fsharp#10769.
This could provide the missing information in the AST.

@nojaf
Copy link
Contributor

nojaf commented Jun 27, 2021

FCS 40, has what we need for this. Looking forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants