Strange behavior with OCaml operators

[abelbraaksma]

Issue created from fantomas-online

Code

module X =
    let (land) x y = x + y
    let foo x y = x land y

Result

Note: this result is fine, using the online version, but it also shows the following message, but without the details, so I don't know if any formatting actually took place:

module X =
    let (land) x y = x + y
    let foo x y = x land y

The result I get locally, with a slightly older version of Fantomas, is:

module X =
    let ``land`` x y = x + y
    let foo x y = x ``land`` y

Problem description

The land etc as an operator was supposed to be deprecated by the F# compiler, but I could still create these without setting any local compiler flags, yet I saw my code auto-formatted as the second example above, which obviously won't compile (I have format-on-save on in VS 2022).

I'd totally understand if this is a non-issue, as it is supposed to be a deprecated construct. I just like to understand the CompilerMessage from the online Fantomas and whether constructs like above are indeed ignored by Fantomas, or treated as what they are: operators.

Extra information

• The formatted result breaks my code: partially, it's unclear what happened in the online version
• Fantomas online throws errors, or resurfaces compiler errors, unclear
• The formatted result gives compiler warnings errors.
• I or my company would be willing to help fix this.

Options

Fantomas master branch at 2022-07-19T14:35:52Z - 0fe6785

Default Fantomas configuration

_{Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file? yes, I know ;)}

[nojaf]

Hello,

When Fantomas parses the initial code, it won't process if there are any errors.
land and will yield a warning but we continue:

fantomas/src/Fantomas.Core/CodeFormatterImpl.fs

Lines 15 to 55 in 0fe6785

	let parse (isSignature: bool) (source: ISourceText) : Async<(ParsedInput * DefineCombination)[]> =
	// First get the syntax tree without any defines
	let baseUntypedTree, baseDiagnostics =
	Fantomas.FCS.Parse.parseFile isSignature source []
	let hashDirectives =
	match baseUntypedTree with
	| ImplFile (ParsedImplFileInput (_, _, directives, _))
	| SigFile (ParsedSigFileInput (_, _, directives, _)) -> directives
	match hashDirectives with
	| [] ->
	async {
	let errors =
	baseDiagnostics
	|> List.filter (fun d -> d.Severity = FSharpDiagnosticSeverity.Error)
	if not errors.IsEmpty then
	raise (FormatException $"Parsing failed with errors: %A{baseDiagnostics}")
	return [| (baseUntypedTree, []) |]
	}
	| hashDirectives ->
	let defineCombinations = Defines.getDefineCombination hashDirectives
	defineCombinations
	|> List.map (fun defineCombination ->
	async {
	let untypedTree, diagnostics =
	Fantomas.FCS.Parse.parseFile isSignature source defineCombination
	let errors =
	diagnostics
	|> List.filter (fun d -> d.Severity = FSharpDiagnosticSeverity.Error)
	if not errors.IsEmpty then
	raise (FormatException $"Parsing failed with errors: %A{diagnostics}")
	return (untypedTree, defineCombination)
	})
	|> Async.Parallel

After formatting the code, we validate the result in:

fantomas/src/Fantomas.Core/Validation.fs

Lines 1 to 53 in 0fe6785

	module internal Fantomas.Core.Validation
	open FSharp.Compiler.Diagnostics
	open FSharp.Compiler.Text
	open Fantomas.FCS.Parse
	open Fantomas.Core.SourceParser
	let private safeToIgnoreWarnings =
	set
	[ "This construct is deprecated: it is only for use in the F# library"
	"Identifiers containing '@' are reserved for use in F# code generation" ]
	// Exception of type 'FSharp.Compiler.DiagnosticsLogger+LibraryUseOnly' was thrown.
	let internal noWarningOrErrorDiagnostics diagnostics =
	let errors =
	diagnostics
	|> List.filter (fun e ->
	match e.Severity with
	| FSharpDiagnosticSeverity.Error -> true
	| FSharpDiagnosticSeverity.Hidden
	| FSharpDiagnosticSeverity.Info -> false
	| FSharpDiagnosticSeverity.Warning -> not (safeToIgnoreWarnings.Contains(e.Message)))
	List.isEmpty errors
	/// Check whether an input string is invalid in F# by looking for errors and warnings in the diagnostics.
	let isValidFSharpCode (isSignature: bool) (source: string) : Async<bool> =
	async {
	// First get the syntax tree without any defines
	let sourceText = SourceText.ofString source
	let baseUntypedTree, baseDiagnostics = parseFile isSignature sourceText []
	let hashDirectives =
	match baseUntypedTree with
	| ImplFile (ParsedImplFileInput (_, _, directives, _))
	| SigFile (ParsedSigFileInput (_, _, directives, _)) -> directives
	match hashDirectives with
	| [] -> return noWarningOrErrorDiagnostics baseDiagnostics
	| hashDirectives ->
	let defineCombinations = Defines.getDefineCombination hashDirectives
	let isValidForCombinations =
	defineCombinations
	|> List.map (fun defineCombination ->
	let _, diagnostics = parseFile isSignature sourceText defineCombination
	noWarningOrErrorDiagnostics diagnostics)
	return Seq.forall id isValidForCombinations
	}

I believe currently that land warning is not added to safeToIgnoreWarnings.
From Fantomas' point of view, this is rather irrelevant, as that problem will have been there, to begin with.

The Exception of type 'FSharp.Compiler.DiagnosticsLogger+UserCompilerMessage' was thrown. part comes from

fantomas/src/Fantomas.FCS/Parse.fs

Lines 1048 to 1052 in 0fe6785

	match p.Exception with
	| :? IndentationProblem as ip -> Some ip.Data1, ip.Data0, Some 58
	| :? SyntaxError as se -> Some se.range, (getSyntaxErrorMessage se.Data0), Some 10
	| :? LibraryUseOnly as luo -> Some luo.range, LibraryUseOnlyE().Format, Some 42
	| _ -> None, p.Exception.Message, None

Where we are currently missing | :? UserCompilerMessage as ucm -> Some ucm.range, ucm.message, Some ucm.number to properly process the information.

And potentially we could add This construct is for ML compatibility. In previous versions of F# 'land' was a reserved keyword but the use of this keyword is now deprecated. You can disable this warning by using '--mlcompatibility' or '--nowarn:62'. to the set of safeToIgnoreWarnings.

Do note that fantomas-tool does not filter the diagnostics in https://github.com/fsprojects/fantomas-tools/blob/16f04a5ff9646af112c81a00e2841212da7b9b27/src/server/FantomasOnlinePreview/FormatCode.fs#L32-L62.

Lastly, previous versions of Fantomas (4.x and lower) used to check if the identifier needs backticks, for performance reasons we no longer have this check. Identifiers are all over the place, so being able to trust the source code has really improved the speed.

We only add backticks when the range and length of the identifier differ exactly 4 units.

fantomas/src/Fantomas.Core/CodePrinter.fs

Lines 417 to 427 in 0fe6785

	and genIdent (ident: Ident) =
	let width = ident.idRange.EndColumn - ident.idRange.StartColumn
	let genIdent =
	if ident.idText.Length + 4 = width then
	// add backticks
	!- $"``{ident.idText}``"
	else
	!-ident.idText
	genTriviaFor Ident_ ident.idRange genIdent

If you are interested, I would accept a PR for some of the pointers above.

[abelbraaksma]

@nojaf, thanks for the insights! For some reason, FCS does not give me warnings about the land etc operators. Though I wouldn't dream of using them anyway, allegedly F# 6 added deprecation warnings and I was testing that out, which led to code being formatted as above, hence the report.

So this is basically already fixed in Fantomas 5, per your new way of testing for `` (double backticks) in code: basically leaving them in place when they're there. That only leaves Fantomas erroring out on the warning for FS0062 (which, again, I didn't get locally for some reason).