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

Fix desugar patterns' bug exposed by FsCheck #167

Merged
merged 3 commits into from
Jun 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Fantomas.Tests/PatternMatchingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,15 @@ let UNIFY_ACCEPT_TAC mvs th (asl, w) =
fun i [] -> INSTANTIATE i th'
"""

[<Test>]
let ``desugared lambdas again``() =
formatSourceString false """
fun P -> T""" config
|> prepend newline
|> should equal """
fun P -> T
"""

[<Test>]
let ``should consume spaces before inserting comments``() =
formatSourceString false """
Expand Down
21 changes: 9 additions & 12 deletions src/Fantomas/CodeFormatter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
7 changes: 0 additions & 7 deletions src/Fantomas/CodeFormatter.fsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 +
Expand Down
1 change: 1 addition & 0 deletions src/Fantomas/Fantomas.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" />
<ItemGroup>
<Compile Include="AssemblyInfo.fs" />
<Compile Include="Utils.fs" />
<Compile Include="TokenMatcher.fs" />
<Compile Include="FormatConfig.fs" />
<Compile Include="SourceParser.fs" />
Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas/FormatConfig.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 14 additions & 5 deletions src/Fantomas/SourceParser.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -616,6 +616,16 @@ let (|Var|_|) = function
Some s
| _ -> None

// Compiler-generated patterns often have "_arg" prefix
let (|ComputerGeneratedVar|_|) = function
| SynExpr.Ident(IdentOrKeyword(OpName s)) when String.startsWithOrdinal "_arg" s ->
Some s
| SynExpr.LongIdent(_, LongIdentWithDots.LongIdentWithDots(LongIdentOrKeyword(OpName s), _), opt, _) ->
match opt with
| Some _ -> Some s
| None -> if String.startsWithOrdinal "_arg" s then Some s else None
| _ -> None

/// Get all application params at once
let (|App|_|) e =
let rec loop = function
Expand Down Expand Up @@ -913,10 +923,9 @@ 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" ->
| SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, DesugaredMatch(ss, e), None)], _, _) ->
Some((s, p)::ss, e)
| SynExpr.Match(_, Var s, [Clause(p, e, None)], _, _) when s.StartsWith "_arg" ->
| SynExpr.Match(_, ComputerGeneratedVar s, [Clause(p, e, None)], _, _) ->
Some([(s, p)], e)
| _ -> None

Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas/SourceTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions src/Fantomas/TokenMatcher.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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..]
Expand Down
15 changes: 15 additions & 0 deletions src/Fantomas/Utils.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[<AutoOpen>]
module internal Fantomas.Utils

open System

[<RequireQualifiedAccess>]
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)
2 changes: 1 addition & 1 deletion src/fantomas.sln
Original file line number Diff line number Diff line change
@@ -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}"
Expand Down