From 81f024320ddaca5c3598a9b895360e5a49b543e2 Mon Sep 17 00:00:00 2001 From: dungpa Date: Tue, 9 Jun 2015 09:55:06 +0200 Subject: [PATCH 1/3] Fix desugar patterns' bug exposed by FsCheck --- src/Fantomas/CodeFormatter.fsx | 7 ------- src/Fantomas/SourceParser.fs | 11 +++++++++++ src/fantomas.sln | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Fantomas/CodeFormatter.fsx b/src/Fantomas/CodeFormatter.fsx index 8d19af9d65..e39e87684b 100644 --- a/src/Fantomas/CodeFormatter.fsx +++ b/src/Fantomas/CodeFormatter.fsx @@ -21,13 +21,6 @@ let test (s : string) = fsi.AddPrinter (fun (p : Microsoft.FSharp.Compiler.Range.pos) -> p.ToString()) fsi.AddPrinter (fun (r : Microsoft.FSharp.Compiler.Range.range) -> r.ToString()) -// A bug exposed by FsCheck -parse false """ -fun P -> R -d -s -""";; - // FAILS - sticky-right comment becomes sticky-left test """ 1 + diff --git a/src/Fantomas/SourceParser.fs b/src/Fantomas/SourceParser.fs index df9b2998c8..0588ff4266 100644 --- a/src/Fantomas/SourceParser.fs +++ b/src/Fantomas/SourceParser.fs @@ -616,6 +616,13 @@ let (|Var|_|) = function Some s | _ -> None +let (|ComputerGeneratedVar|_|) = function + | SynExpr.Ident(IdentOrKeyword(OpName s)) -> + None + | SynExpr.LongIdent(_, LongIdentWithDots.LongIdentWithDots(LongIdentOrKeyword(OpName s), _), opt, _) -> + opt |> Option.map (fun _ -> s) + | _ -> None + /// Get all application params at once let (|App|_|) e = let rec loop = function @@ -918,6 +925,10 @@ let rec private (|DesugaredMatch|_|) = function Some((s, p)::ss, e) | SynExpr.Match(_, Var s, [Clause(p, e, None)], _, _) when s.StartsWith "_arg" -> Some([(s, p)], e) + | SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, DesugaredMatch(ss, e), None)], _, _) -> + Some((s, p)::ss, e) + | SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, e, None)], _, _) -> + Some([(s, p)], e) | _ -> None type ComplexPat = diff --git a/src/fantomas.sln b/src/fantomas.sln index 678475a134..dc4aaaf292 100644 --- a/src/fantomas.sln +++ b/src/fantomas.sln @@ -1,6 +1,6 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 2012 +# Visual Studio 2013 Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "Fantomas", "Fantomas\Fantomas.fsproj", "{919ADF2D-DF2E-4F4D-9F8B-0376346535C9}" EndProject Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "Fantomas.Tests", "Fantomas.Tests\Fantomas.Tests.fsproj", "{28E90D4B-75CE-48D3-ACFE-3AB37179233B}" From 20e8e86c35b720ae6e864c1b57ac11926accc756 Mon Sep 17 00:00:00 2001 From: dungpa Date: Tue, 9 Jun 2015 12:23:08 +0200 Subject: [PATCH 2/3] Refactor and add tests --- src/Fantomas.Tests/PatternMatchingTests.fs | 9 +++++++++ src/Fantomas/SourceParser.fs | 12 ++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Fantomas.Tests/PatternMatchingTests.fs b/src/Fantomas.Tests/PatternMatchingTests.fs index 1405490fde..bbd84579a1 100644 --- a/src/Fantomas.Tests/PatternMatchingTests.fs +++ b/src/Fantomas.Tests/PatternMatchingTests.fs @@ -232,6 +232,15 @@ let UNIFY_ACCEPT_TAC mvs th (asl, w) = fun i [] -> INSTANTIATE i th' """ +[] +let ``desugared lambdas again``() = + formatSourceString false """ +fun P -> T""" config + |> prepend newline + |> should equal """ +fun P -> T +""" + [] let ``should consume spaces before inserting comments``() = formatSourceString false """ diff --git a/src/Fantomas/SourceParser.fs b/src/Fantomas/SourceParser.fs index 0588ff4266..415a692686 100644 --- a/src/Fantomas/SourceParser.fs +++ b/src/Fantomas/SourceParser.fs @@ -616,11 +616,12 @@ let (|Var|_|) = function Some s | _ -> None +// Compiler-generated patterns often have "_arg" prefix let (|ComputerGeneratedVar|_|) = function - | SynExpr.Ident(IdentOrKeyword(OpName s)) -> - None + | SynExpr.Ident(IdentOrKeyword(OpName s)) when s.StartsWith "_arg" -> + Some s | SynExpr.LongIdent(_, LongIdentWithDots.LongIdentWithDots(LongIdentOrKeyword(OpName s), _), opt, _) -> - opt |> Option.map (fun _ -> s) + | None -> if s.StartsWith "_arg" then Some s else None | _ -> None /// Get all application params at once @@ -920,11 +921,6 @@ let (|RecordField|) = function let (|Clause|) (SynMatchClause.Clause(p, eo, e, _, _)) = (p, e, eo) let rec private (|DesugaredMatch|_|) = function - // Compiler-generated patterns has "_arg" prefix - | SynExpr.Match(_, Var s, [Clause(p, DesugaredMatch(ss, e), None)], _, _) when s.StartsWith "_arg" -> - Some((s, p)::ss, e) - | SynExpr.Match(_, Var s, [Clause(p, e, None)], _, _) when s.StartsWith "_arg" -> - Some([(s, p)], e) | SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, DesugaredMatch(ss, e), None)], _, _) -> Some((s, p)::ss, e) | SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, e, None)], _, _) -> From b8e0b7a2ee2191dab6f4fd9dc6532b78e86767bd Mon Sep 17 00:00:00 2001 From: dungpa Date: Tue, 9 Jun 2015 13:00:58 +0200 Subject: [PATCH 3/3] Refactor string functions --- src/Fantomas/CodeFormatter.fs | 21 +++++++++------------ src/Fantomas/Fantomas.fsproj | 1 + src/Fantomas/FormatConfig.fs | 4 ++-- src/Fantomas/SourceParser.fs | 10 ++++++---- src/Fantomas/SourceTransformer.fs | 2 +- src/Fantomas/TokenMatcher.fs | 10 +++++----- src/Fantomas/Utils.fs | 15 +++++++++++++++ 7 files changed, 39 insertions(+), 24 deletions(-) create mode 100644 src/Fantomas/Utils.fs diff --git a/src/Fantomas/CodeFormatter.fs b/src/Fantomas/CodeFormatter.fs index b545b7149d..ee94fd13f9 100644 --- a/src/Fantomas/CodeFormatter.fs +++ b/src/Fantomas/CodeFormatter.fs @@ -319,14 +319,11 @@ let isValidFSharpCode isFsiFile sourceCode = isValidAST (parse isFsiFile sourceCode) with _ -> false -let internal split (str : string) = - str.Replace("\r\n", "\n").Replace("\r", "\n").Split('\n') - let internal formatWith ast input config = // Use '\n' as the new line delimiter consistently // It would be easier for F# parser let sourceCode = defaultArg input "" - let normalizedSourceCode = sourceCode.Replace("\r\n", "\n").Replace("\r", "\n") + let normalizedSourceCode = String.normalizeNewLine sourceCode let formattedSourceCode = Context.create config normalizedSourceCode |> genParsedInput ASTContext.Default ast @@ -455,7 +452,7 @@ let internal formatRange returnFormattedContentOnly isFsiFile (range : range) (l let sel = sourceCode.[start..finish].TrimEnd('\r') if startWithMember sel then (String.Join(String.Empty, "type T = ", Environment.NewLine, new String(' ', startCol), sel), TypeMember) - elif sel.TrimStart().StartsWith("and") then + elif String.startsWithOrdinal "and" (sel.TrimStart()) then let p = getPatch startCol lines.[..startLine-1] let pattern = Regex("and") let replacement = @@ -472,7 +469,7 @@ let internal formatRange returnFormattedContentOnly isFsiFile (range : range) (l let post = if finish < sourceCode.Length then let post = sourceCode.[finish+1..] - if post.StartsWith("\n") then Environment.NewLine + post.[1..] + if String.startsWithOrdinal "\n" post then Environment.NewLine + post.[1..] else post else String.Empty @@ -505,7 +502,7 @@ let internal formatRange returnFormattedContentOnly isFsiFile (range : range) (l // Get formatted selection with "type T = \n" patch let result = formatSelection isFsiFile selection config // Remove the patch - let contents = split result + let contents = String.normalizeThenSplitNewLine result if Array.isEmpty contents then if returnFormattedContentOnly then result else @@ -522,17 +519,17 @@ let internal formatRange returnFormattedContentOnly isFsiFile (range : range) (l let result = formatSelection isFsiFile selection config // Substitute by old contents let pattern = if patch = RecType then Regex("type") else Regex("let rec") - let formatteds = split (pattern.Replace(result, "and", 1)) + let formatteds = String.normalizeThenSplitNewLine (pattern.Replace(result, "and", 1)) reconstructSourceCode startCol formatteds pre post | Nothing -> let result = formatSelection isFsiFile selection config - let formatteds = split result + let formatteds = String.normalizeThenSplitNewLine result reconstructSourceCode startCol formatteds pre post /// Format a part of source string using given config, and return the (formatted) selected part only. /// Beware that the range argument is inclusive. If the range has a trailing newline, it will appear in the formatted result. let formatSelectionOnly isFsiFile (range : range) (sourceCode : string) config = - let lines = split sourceCode + let lines = String.normalizeThenSplitNewLine sourceCode // Move to the section with real contents let contentRange = @@ -577,7 +574,7 @@ let formatSelectionOnly isFsiFile (range : range) (sourceCode : string) config = /// Format a selected part of source string using given config; expanded selected ranges to parsable ranges. let formatSelectionExpanded isFsiFile (range : range) (sourceCode : string) config = - let lines = split sourceCode + let lines = String.normalizeThenSplitNewLine sourceCode let sourceTokenizer = SourceTokenizer([], "/tmp.fsx") // Move to the section with real contents @@ -619,7 +616,7 @@ let makePos line col = mkPos line col /// Infer selection around cursor by looking for a pair of '[' and ']', '{' and '}' or '(' and ')'. let inferSelectionFromCursorPos (cursorPos : pos) (sourceCode : string) = - let lines = split sourceCode + let lines = String.normalizeThenSplitNewLine sourceCode let sourceTokenizer = SourceTokenizer([], "/tmp.fsx") let openDelimiters = dict ["[", List; "[|", Array; "{", SequenceOrRecord; "(", Tuple] let closeDelimiters = dict ["]", List; "|]", Array; "}", SequenceOrRecord; ")", Tuple] diff --git a/src/Fantomas/Fantomas.fsproj b/src/Fantomas/Fantomas.fsproj index 32a7365a77..0899ad7d28 100644 --- a/src/Fantomas/Fantomas.fsproj +++ b/src/Fantomas/Fantomas.fsproj @@ -56,6 +56,7 @@ + diff --git a/src/Fantomas/FormatConfig.fs b/src/Fantomas/FormatConfig.fs index 290d4e0c61..2e3822347a 100644 --- a/src/Fantomas/FormatConfig.fs +++ b/src/Fantomas/FormatConfig.fs @@ -7,7 +7,7 @@ open System.Text.RegularExpressions open System.CodeDom.Compiler open Microsoft.FSharp.Compiler.Range - +open Fantomas open Fantomas.TokenMatcher type FormatException(msg : string) = @@ -137,7 +137,7 @@ type internal Context = Content = ""; Positions = [||]; Comments = Dictionary(); Directives = Dictionary() } static member create config (content : string) = - let content = content.Replace("\r\n", "\n").Replace("\r", "\n") + let content = String.normalizeNewLine content let positions = content.Split('\n') |> Seq.map (fun s -> String.length s + 1) diff --git a/src/Fantomas/SourceParser.fs b/src/Fantomas/SourceParser.fs index 415a692686..e9906dba31 100644 --- a/src/Fantomas/SourceParser.fs +++ b/src/Fantomas/SourceParser.fs @@ -56,7 +56,7 @@ let (|LongIdent|) (li : LongIdent) = |> String.concat "." |> fun s -> // Assume that if it starts with base, it's going to be the base keyword - if s.StartsWith "``base``." then + if String.startsWithOrdinal "``base``." s then String.Join("", "base.", s.[9..]) else s @@ -99,7 +99,7 @@ let (|OpNameFull|) (x : Identifier) = let s' = DecompileOpName s if IsActivePatternName s || IsInfixOperator s || IsPrefixOperator s || IsTernaryOperator s || s = "op_Dynamic" then /// Use two spaces for symmetry - if s'.StartsWith "*" && s' <> "*" then + if String.startsWithOrdinal "*" s' && s' <> "*" then sprintf "( %s )" s' else sprintf "(%s)" s' else @@ -618,10 +618,12 @@ let (|Var|_|) = function // Compiler-generated patterns often have "_arg" prefix let (|ComputerGeneratedVar|_|) = function - | SynExpr.Ident(IdentOrKeyword(OpName s)) when s.StartsWith "_arg" -> + | SynExpr.Ident(IdentOrKeyword(OpName s)) when String.startsWithOrdinal "_arg" s -> Some s | SynExpr.LongIdent(_, LongIdentWithDots.LongIdentWithDots(LongIdentOrKeyword(OpName s), _), opt, _) -> - | None -> if s.StartsWith "_arg" then Some s else None + match opt with + | Some _ -> Some s + | None -> if String.startsWithOrdinal "_arg" s then Some s else None | _ -> None /// Get all application params at once diff --git a/src/Fantomas/SourceTransformer.fs b/src/Fantomas/SourceTransformer.fs index 367cd65225..85336cc910 100644 --- a/src/Fantomas/SourceTransformer.fs +++ b/src/Fantomas/SourceTransformer.fs @@ -113,7 +113,7 @@ let genConst (Unresolved(c, r, s)) = /// Check whether a range starting with a specified token let startWith prefix (r : range) ctx = - lookup r ctx |> Option.exists (fun s -> s.StartsWith(prefix)) + lookup r ctx |> Option.exists (String.startsWithOrdinal prefix) // A few active patterns for printing purpose diff --git a/src/Fantomas/TokenMatcher.fs b/src/Fantomas/TokenMatcher.fs index 32f7dd9f6c..d3e137fa32 100644 --- a/src/Fantomas/TokenMatcher.fs +++ b/src/Fantomas/TokenMatcher.fs @@ -24,7 +24,7 @@ type Token = let tokenize defines (content : string) = seq { let sourceTokenizer = SourceTokenizer(defines, "/tmp.fsx") - let lines = content.Replace("\r\n","\n").Split('\r', '\n') + let lines = String.normalizeThenSplitNewLine content let lexState = ref 0L for (i, line) in lines |> Seq.zip [1..lines.Length] do let lineTokenizer = sourceTokenizer.CreateLineTokenizer line @@ -501,7 +501,7 @@ let integrateComments (originalText : string) (newText : string) = addText Environment.NewLine for x in tokensText do addText x let moreNewTokens = - if text.StartsWith("#endif") then + if String.startsWithOrdinal "#endif" text then match newTokens with | WhiteSpaces(ws, moreNewTokens) -> // There are some whitespaces, use them up @@ -513,7 +513,7 @@ let integrateComments (originalText : string) (newText : string) = restoreIndent id newTokens | [] -> [] - else if text.StartsWith("#if") then + elif String.startsWithOrdinal "#if" text then // Save current indentation for #else branch let indent = getIndent newTokens saveIndent indent @@ -529,13 +529,13 @@ let integrateComments (originalText : string) (newText : string) = | (InactiveCodeChunk (tokensText, moreOrigTokens)), _ -> Debug.WriteLine("injecting inactive code '{0}'", String.concat "" tokensText |> box) let text = String.concat "" tokensText - let lines = text.Replace("\r\n", "\n").Replace("\r", "\n").Split([|'\n'|], StringSplitOptions.RemoveEmptyEntries) + let lines = (String.normalizeNewLine text).Split([|'\n'|], StringSplitOptions.RemoveEmptyEntries) // What is current indentation of this chunk let numSpaces = countStartingSpaces lines Debug.WriteLine("the number of starting spaces is {0}", numSpaces) // Write the chunk in the same indentation with #if branch for line in lines do - if line.[numSpaces..].StartsWith("#") then + if String.startsWithOrdinal "#" line.[numSpaces..] then // Naive recognition of inactive preprocessors addText Environment.NewLine addText line.[numSpaces..] diff --git a/src/Fantomas/Utils.fs b/src/Fantomas/Utils.fs new file mode 100644 index 0000000000..002b20b7d5 --- /dev/null +++ b/src/Fantomas/Utils.fs @@ -0,0 +1,15 @@ +[] +module internal Fantomas.Utils + +open System + +[] +module String = + let normalizeNewLine (str : string) = + str.Replace("\r\n", "\n").Replace("\r", "\n") + + let normalizeThenSplitNewLine (str : string) = + (normalizeNewLine str).Split('\n') + + let startsWithOrdinal (prefix : string) (str : string) = + str.StartsWith(prefix, StringComparison.Ordinal) \ No newline at end of file