From b5c6d7f78afeba248dabbb4bd6a16a7ced76cfe0 Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 22 Apr 2021 14:41:24 +0200 Subject: [PATCH] Optimize comment after source code detection. Fixes #1671. --- src/Fantomas.Tests/CommentTests.fs | 172 +++++++++++++++++ src/Fantomas.Tests/TriviaTests.fs | 7 +- src/Fantomas/CodePrinter.fs | 6 +- src/Fantomas/TokenParser.fs | 292 ++++++++++++++++------------- src/Fantomas/TriviaTypes.fs | 3 +- 5 files changed, 344 insertions(+), 136 deletions(-) diff --git a/src/Fantomas.Tests/CommentTests.fs b/src/Fantomas.Tests/CommentTests.fs index c6b919b454..b7446908fd 100644 --- a/src/Fantomas.Tests/CommentTests.fs +++ b/src/Fantomas.Tests/CommentTests.fs @@ -1311,3 +1311,175 @@ let a = b (* meh *) let a = b """ + +[] +let ``comment right after first token`` () = + formatSourceString + false + """ +1// +// next line +""" + config + |> prepend newline + |> should + equal + """ +1 // +// next line +""" + +[] +[] +let ``block comment followed by line comment`` () = + formatSourceString + false + """ +(* foo *)// bar +let a = 0 +""" + config + |> prepend newline + |> should + equal + """ +(* foo *) // bar +let a = 0 +""" + +[] +let ``line comment after source code`` () = + formatSourceString + false + """ +__SOURCE_DIRECTORY__ // comment +""" + config + |> prepend newline + |> should + equal + """ +__SOURCE_DIRECTORY__ // comment +""" + +[] +let ``line comment after hash define`` () = + formatSourceString + false + """ +#if FOO // MEH +#endif +""" + config + |> prepend newline + |> should + equal + """ +#if FOO // MEH +#endif +""" + +[] +let ``line comment after interpolated string`` () = + formatSourceString + false + """ +$"{meh}.." // foo +""" + config + |> prepend newline + |> should + equal + """ +$"{meh}.." // foo +""" + +[] +let ``line comment after negative constant`` () = + formatSourceString + false + """ +-1.0 // foo +""" + config + |> prepend newline + |> should + equal + """ +-1.0 // foo +""" + +[] +let ``line comment after trivia number`` () = + formatSourceString + false + """ +1. // bar +""" + config + |> prepend newline + |> should + equal + """ +1. // bar +""" + +[] +let ``line comment after infix operator in full words`` () = + formatSourceString + false + """ +op_LessThan // meh +""" + config + |> prepend newline + |> should + equal + """ +op_LessThan // meh +""" + +[] +let ``line comment after ident between ticks`` () = + formatSourceString + false + """ +``foo oo`` // bar +""" + config + |> prepend newline + |> should + equal + """ +``foo oo`` // bar +""" + +[] +let ``line comment after special char`` () = + formatSourceString + false + """ +'\u0000' // foo +""" + config + |> prepend newline + |> should + equal + """ +'\u0000' // foo +""" + +[] +let ``line comment after embedded il`` () = + formatSourceString + false + """ +(# "" x : 'U #) // bar +""" + config + |> prepend newline + |> should + equal + """ +(# "" x : 'U #) // bar +""" diff --git a/src/Fantomas.Tests/TriviaTests.fs b/src/Fantomas.Tests/TriviaTests.fs index b66f537cd8..0003ebd9a1 100644 --- a/src/Fantomas.Tests/TriviaTests.fs +++ b/src/Fantomas.Tests/TriviaTests.fs @@ -247,10 +247,9 @@ x let triviaNodes = toTrivia source |> List.head match triviaNodes with - | [ { ContentBefore = [ Comment (BlockComment (fooComment, _, true)); Comment (BlockComment (barComment, _, true)) ] } ] -> - fooComment == "(* foo *)" - barComment == "(* bar *)" - | _ -> fail () + | [ { ContentBefore = [ Comment (BlockComment (combinedComment, _, true)) ] } ] -> + combinedComment == "(* foo *)\n(* bar *)" + | _ -> Assert.Fail(sprintf "Unexpected trivia %A" triviaNodes) [] let ``block comment inside line comment parsed correctly`` () = diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs index a581456a3b..0537dc2b21 100644 --- a/src/Fantomas/CodePrinter.fs +++ b/src/Fantomas/CodePrinter.fs @@ -1602,7 +1602,7 @@ and genExpr astContext synExpr ctx = +> sepSpace +> genExpr astContext e - | Paren (_, ILEmbedded r, _, _) -> + | Paren (_, ILEmbedded r, rpr, _) -> fun ctx -> let expr = Map.tryFindOrEmptyList SynExpr_LibraryOnlyILAssembly ctx.TriviaMainNodes @@ -1618,7 +1618,9 @@ and genExpr astContext synExpr ctx = |> Option.map (!-) |> Option.defaultValue sepNone - expr ctx + (expr + +> optSingle (fun r -> leaveNodeTokenByName r RPAREN) rpr) + ctx | Paren (lpr, e, rpr, pr) -> match e with | LetOrUses _ -> diff --git a/src/Fantomas/TokenParser.fs b/src/Fantomas/TokenParser.fs index 16305d7703..76f04706a4 100644 --- a/src/Fantomas/TokenParser.fs +++ b/src/Fantomas/TokenParser.fs @@ -11,6 +11,7 @@ let private whiteSpaceTag = 4 let private lineCommentTag = 8 let private commentTag = 3 let private greaterTag = 160 +let private identTag = 191 // workaround for cases where tokenizer dont output "delayed" part of operator after ">." // See https://github.com/fsharp/FSharp.Compiler.Service/issues/874 @@ -41,14 +42,12 @@ let rec private tokenizeLine (tokenizer: FSharpLineTokenizer) sourceCodeLines st let extraToken = { TokenInfo = extraTokenInfo LineNumber = lineNumber - Content = getTokenText sourceCodeLines lineNumber extraTokenInfo - Index = 0 } + Content = getTokenText sourceCodeLines lineNumber extraTokenInfo } let token = { TokenInfo = tok LineNumber = lineNumber - Content = getTokenText sourceCodeLines lineNumber tok - Index = 0 } + Content = getTokenText sourceCodeLines lineNumber tok } tokenizeLine tokenizer sourceCodeLines state lineNumber (token :: extraToken :: tokens) @@ -56,8 +55,7 @@ let rec private tokenizeLine (tokenizer: FSharpLineTokenizer) sourceCodeLines st let token : Token = { TokenInfo = tok LineNumber = lineNumber - Content = getTokenText sourceCodeLines lineNumber tok - Index = 0 } + Content = getTokenText sourceCodeLines lineNumber tok } // Tokenize the rest, in the new state tokenizeLine tokenizer sourceCodeLines state lineNumber (token :: tokens) @@ -86,7 +84,6 @@ let private createHashToken lineNumber content offset = { LineNumber = lineNumber Content = content - Index = 0 TokenInfo = { TokenName = "HASH_IF" LeftColumn = left @@ -343,19 +340,15 @@ and tokenize defines (hashTokens: Token list) (content: string) : Token list = |> List.map (fun t -> t.LineNumber) |> List.distinct - let combined = - if List.isNotEmpty hashTokens then - let filteredHashes = - hashTokens - |> List.filter (fun t -> not (List.contains t.LineNumber existingLines)) - // filter hashes that are present in source code parsed by the Tokenizer. - tokens @ filteredHashes - |> List.sortBy (fun t -> t.LineNumber, t.TokenInfo.LeftColumn) - else - tokens - - combined - |> List.mapi (fun idx t -> { t with Index = idx }) + if List.isNotEmpty hashTokens then + let filteredHashes = + hashTokens + |> List.filter (fun t -> not (List.contains t.LineNumber existingLines)) + // filter hashes that are present in source code parsed by the Tokenizer. + tokens @ filteredHashes + |> List.sortBy (fun t -> t.LineNumber, t.TokenInfo.LeftColumn) + else + tokens let getDefinesWords (tokens: Token list) = tokens @@ -492,6 +485,17 @@ let private isOperatorOrKeyword { TokenInfo = { CharClass = cc } } = cc = FSharpTokenCharKind.Keyword || cc = FSharpTokenCharKind.Operator +let private (|KeywordOrOperatorToken|_|) (token: Token) = + let isOperatorOrKeyword = isOperatorOrKeyword token + + let isKnownKeywordTrivia () = + List.exists (fun k -> token.TokenInfo.TokenName = k) keywordTrivia + + if isOperatorOrKeyword && isKnownKeywordTrivia () then + Some token + else + None + let private onlyNumberRegex = System.Text.RegularExpressions.Regex(@"^\d+$") @@ -595,11 +599,27 @@ let private (|StringText|_|) tokens = | _ -> None let private identIsDecompiledOperator (token: Token) = - let decompiledName = + let decompiledName () = PrettyNaming.DecompileOpName token.Content - token.TokenInfo.TokenName = "IDENT" - && decompiledName <> token.Content + token.TokenInfo.Tag = identTag + && (decompiledName () <> token.Content) + +let private (|DecompiledOperatorToken|_|) (token: Token) = + if identIsDecompiledOperator token then + Some token + else + None + +let private (|IdentBetweenTicksToken|_|) (token: Token) = + if + token.TokenInfo.Tag = identTag + && token.Content.StartsWith("``") + && token.Content.EndsWith("``") + then + Some token + else + None let private extractContentPreservingNewLines (tokens: Token list) = let rec loop result = @@ -629,7 +649,7 @@ let ``only whitespaces were found in the remainder of the line`` lineNumber toke && t.TokenInfo.Tag <> whiteSpaceTag) |> not -let private (|LineComment|_|) (token: Token) = +let private (|LineCommentToken|_|) (token: Token) = if token.TokenInfo.Tag = lineCommentTag then Some token else @@ -642,12 +662,24 @@ let private (|NoCommentToken|_|) (token: Token) = else None +let private (|CommentToken|_|) (token: Token) = + if token.TokenInfo.Tag = commentTag then + Some token + else + None + let private (|WhiteSpaceToken|_|) (token: Token) = if token.TokenInfo.Tag = whiteSpaceTag then Some token else None +let private (|NonWhiteSpaceToken|_|) (token: Token) = + if token.TokenInfo.Tag <> whiteSpaceTag then + Some token + else + None + let private (|SemicolonToken|_|) (token: Token) = if token.TokenInfo.Tag = 83 then Some token @@ -655,16 +687,20 @@ let private (|SemicolonToken|_|) (token: Token) = None let private (|LineComments|_|) (tokens: Token list) = - let rec collect (tokens: Token list) (lastLineNumber: int) (commentTokens: Token list) = + let rec collect + (tokens: Token list) + (lastLineNumber: int) + (finalContinuation: Token list -> Token list) + : Token list * Token list = match tokens with - | LineComment lc :: rest when (lc.LineNumber <= lastLineNumber + 1) -> - collect rest lc.LineNumber (lc :: commentTokens) - | _ -> commentTokens + | LineCommentToken lc :: rest when (lc.LineNumber <= lastLineNumber + 1) -> + collect rest lc.LineNumber (fun commentTokens -> lc :: commentTokens |> finalContinuation) + | _ -> finalContinuation [], tokens match tokens with - | LineComment h :: _ -> - collect tokens h.LineNumber [] - |> fun commentTokens -> Some(List.rev commentTokens, List.skip commentTokens.Length tokens) + | LineCommentToken h :: _ -> + let commentTokens, rest = collect tokens h.LineNumber id + Some(commentTokens, rest) | _ -> None let private collectComment (commentTokens: Token list) = @@ -727,41 +763,58 @@ let private (|KeywordString|_|) (token: Token) = else None +let private (|BlockCommentTokens|_|) (tokens: Token list) = + let rec collectTokens (rest: Token list) (finalContinuation: Token list -> Token list) : Token list * Token list = + match rest with + | CommentToken ct :: rest -> collectTokens rest (fun commentTokens -> ct :: commentTokens |> finalContinuation) + | _ -> finalContinuation [], rest + + match tokens with + | CommentToken { Content = "(*" } :: _ -> + let comments, rest = collectTokens tokens id + Some(comments, rest) + | _ -> None + +let private (|MinusToken|_|) (token: Token) = + if token.TokenInfo.Tag = 62 then + Some token + else + None + +let private (|NumberToken|_|) (token: Token) = + if isNumber token then + Some token + else + None + +let rec private lastTwoItems + (project: 't -> 'ret) + (fallbackLastButOne: 'ret) + (fallbackLast: 'ret) + (items: 't list) + : 'ret * 'ret = + match items with + | [ f; s ] -> project f, project s + | [ s ] -> fallbackLast, project s + | [] -> fallbackLastButOne, fallbackLast + | _ :: tail -> lastTwoItems project fallbackLastButOne fallbackLast tail + let rec private getTriviaFromTokensThemSelves (mkRange: MkRange) - (allTokens: Token list) + (lastButOneNonWhiteSpaceToken: Token option) + (lastNonWhiteSpaceToken: Token option) (tokens: Token list) foundTrivia = match tokens with - | LineComments (({ Index = headIndex - LineNumber = headLineNumber } :: _ as commentTokens), - nextTokens) -> - let prevToken = List.tryItem (headIndex - 1) allTokens - let prevButOneToken = List.tryItem (headIndex - 2) allTokens - + | LineComments ({ LineNumber = headLineNumber } :: _ as commentTokens, rest) -> let isAfterSourceCode = - match prevButOneToken, prevToken with - | Some (SemicolonToken sc), Some (WhiteSpaceToken _) when sc.LineNumber = headLineNumber -> - let tokensOfSameLine = - let rec collect (index: int) (acc: Token list) : Token list = - match List.tryItem index allTokens with - | Some t when (t.LineNumber = headLineNumber) -> collect (index - 1) (t :: acc) - | _ -> acc - - collect (headIndex - 3) [] - - match tokensOfSameLine with - | [] - // WHITESPACE SEMICOLON WHITESPACE LINE_COMMENT, see https://github.com/fsprojects/fantomas/issues/1643 - | [ WhiteSpaceToken _ ] -> false - | _ -> true - | Some { LineNumber = ncln }, Some (WhiteSpaceToken _) -> - // IDENT WHITESPACE LINE_COMMENT - ncln = headLineNumber - | Some { LineNumber = l1 }, Some { LineNumber = l2 } -> - // IDENT LINE_COMMENT - headLineNumber = l1 && headLineNumber = l2 + match lastButOneNonWhiteSpaceToken, lastNonWhiteSpaceToken with + | Some otherLineToken, Some (SemicolonToken sc) when otherLineToken.LineNumber <> sc.LineNumber -> + // IDENT SEMICOLON LINE_COMMENT + // See https://github.com/fsprojects/fantomas/issues/1643 + false + | _, Some t -> headLineNumber = t.LineNumber | _ -> false let info = @@ -829,29 +882,12 @@ let rec private getTriviaFromTokensThemSelves (Trivia.Create triviaContent range :: foundTrivia) - getTriviaFromTokensThemSelves mkRange allTokens nextTokens info - - | headToken :: rest when - (headToken.TokenInfo.TokenName = "COMMENT" - && headToken.Content = "(*") - -> - let blockCommentTokens = - rest - |> List.takeWhileState - (fun depth t -> - let newDepth = - match t.Content with - | "(*" -> depth + 1 - | "*)" -> depth - 1 - | _ -> depth - - newDepth, t.TokenInfo.TokenName = "COMMENT" && depth > 0) - 1 + getTriviaFromTokensThemSelves mkRange lastButOneNonWhiteSpaceToken lastNonWhiteSpaceToken rest info + | BlockCommentTokens (headToken :: _ as blockCommentTokens, rest) -> let comment = let groupedByLineNumber = - headToken - |> List.prependItem blockCommentTokens + blockCommentTokens |> List.groupBy (fun t -> t.LineNumber) let newLines = @@ -870,19 +906,17 @@ let rec private getTriviaFromTokensThemSelves |> String.concat Environment.NewLine |> String.normalizeNewLine - let nextTokens = - List.length blockCommentTokens - |> fun length -> List.skip length rest + let lastButOne, lastToken = + lastTwoItems Some lastNonWhiteSpaceToken (Some headToken) blockCommentTokens let range = - let lastToken = List.last blockCommentTokens - getRangeBetween mkRange headToken lastToken + getRangeBetween mkRange headToken (Option.defaultValue headToken lastToken) let info = Trivia.Create(Comment(BlockComment(comment, false, false))) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens nextTokens info + getTriviaFromTokensThemSelves mkRange lastButOne lastToken rest info | KeywordString ks :: rest -> let range = getRangeBetween mkRange ks ks @@ -891,20 +925,16 @@ let rec private getTriviaFromTokensThemSelves Trivia.Create(KeywordString(ks.Content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some ks) rest info - | headToken :: rest when - (isOperatorOrKeyword headToken - && List.exists (fun k -> headToken.TokenInfo.TokenName = k) keywordTrivia) - -> - let range = - getRangeBetween mkRange headToken headToken + | KeywordOrOperatorToken koo :: rest -> + let range = getRangeBetween mkRange koo koo let info = - Trivia.Create(Keyword(headToken)) range + Trivia.Create(Keyword(koo)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some koo) rest info | HashTokens (hashTokens, rest) -> let directiveContent = @@ -933,7 +963,7 @@ let rec private getTriviaFromTokensThemSelves Trivia.Create(Directive(directiveContent)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastButOneNonWhiteSpaceToken lastNonWhiteSpaceToken rest info | EndOfInterpolatedString (stringTokens, interpStringEnd, rest) -> let stringContent = @@ -961,14 +991,17 @@ let rec private getTriviaFromTokensThemSelves Trivia.Create(StringContent(stringContent)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + let prevButOne, prev = + List.tryLast stringTokens, Some interpStringEnd + + getTriviaFromTokensThemSelves mkRange prevButOne prev rest info | StringText (head, stringTokens, rest, stringContent) -> - let lastToken = - List.tryLast stringTokens - |> Option.defaultValue head + let lastButOne, lastToken = + lastTwoItems Some None (Some head) stringTokens - let range = getRangeBetween mkRange head lastToken + let range = + getRangeBetween mkRange head (Option.defaultValue head lastToken) let info = Trivia.Create(StringContent(stringContent)) range @@ -979,50 +1012,43 @@ let rec private getTriviaFromTokensThemSelves | [] -> [] | _ -> List.skip (List.length stringTokens - 1) rest - getTriviaFromTokensThemSelves mkRange allTokens nextRest info + getTriviaFromTokensThemSelves mkRange lastButOne lastToken nextRest info - | minus :: head :: rest when - (minus.TokenInfo.TokenName = "MINUS" - && isNumber head) - -> - let range = getRangeBetween mkRange minus head + | MinusToken minus :: NumberToken number :: rest -> + let range = getRangeBetween mkRange minus number let info = - Trivia.Create(Number(minus.Content + head.Content)) range + Trivia.Create(Number(minus.Content + number.Content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange (Some minus) (Some number) rest info - | head :: rest when (isNumber head) -> - let range = getRangeForSingleToken mkRange head + | NumberToken number :: rest -> + let range = getRangeForSingleToken mkRange number let info = - Trivia.Create(Number(head.Content)) range + Trivia.Create(Number(number.Content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some number) rest info - | head :: rest when (identIsDecompiledOperator head) -> - let range = getRangeBetween mkRange head head + | DecompiledOperatorToken ident :: rest -> + let range = getRangeBetween mkRange ident ident let info = - Trivia.Create(IdentOperatorAsWord head.Content) range + Trivia.Create(IdentOperatorAsWord ident.Content) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some ident) rest info - | head :: rest when - (head.TokenInfo.TokenName = "IDENT" - && head.Content.StartsWith("``") - && head.Content.EndsWith("``")) - -> - let range = getRangeBetween mkRange head head + | IdentBetweenTicksToken ident :: rest -> + let range = getRangeBetween mkRange ident ident let info = - Trivia.Create(IdentBetweenTicks(head.Content)) range + Trivia.Create(IdentBetweenTicks(ident.Content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some ident) rest info | CharToken head :: rest -> let range = getRangeBetween mkRange head head @@ -1031,7 +1057,7 @@ let rec private getTriviaFromTokensThemSelves Trivia.Create(CharContent(head.Content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info + getTriviaFromTokensThemSelves mkRange lastNonWhiteSpaceToken (Some head) rest info | EmbeddedILTokens (embeddedTokens, rest) -> let content = @@ -1049,8 +1075,18 @@ let rec private getTriviaFromTokensThemSelves Trivia.Create(EmbeddedIL(content)) range |> List.prependItem foundTrivia - getTriviaFromTokensThemSelves mkRange allTokens rest info - | _ :: rest -> getTriviaFromTokensThemSelves mkRange allTokens rest foundTrivia + let prevButOne, prev = + lastTwoItems Some lastButOneNonWhiteSpaceToken lastNonWhiteSpaceToken embeddedTokens + + getTriviaFromTokensThemSelves mkRange prevButOne prev rest info + + | NonWhiteSpaceToken h :: rest -> + let prevButOne = lastNonWhiteSpaceToken + let prev = Some h + getTriviaFromTokensThemSelves mkRange prevButOne prev rest foundTrivia + + | _ :: rest -> + getTriviaFromTokensThemSelves mkRange lastButOneNonWhiteSpaceToken lastNonWhiteSpaceToken rest foundTrivia | [] -> foundTrivia @@ -1087,7 +1123,7 @@ let private findEmptyNewlinesInTokens let getTriviaFromTokens (mkRange: MkRange) (tokens: Token list) = let fromTokens = - getTriviaFromTokensThemSelves mkRange tokens tokens [] + getTriviaFromTokensThemSelves mkRange None None tokens [] let isMultilineString (s: string) = s.Contains("\n") diff --git a/src/Fantomas/TriviaTypes.fs b/src/Fantomas/TriviaTypes.fs index 1236a9b220..0dbc061653 100644 --- a/src/Fantomas/TriviaTypes.fs +++ b/src/Fantomas/TriviaTypes.fs @@ -58,8 +58,7 @@ type FsTokenType = type Token = { TokenInfo: FSharpTokenInfo LineNumber: int - Content: string - Index: int } + Content: string } type Comment = | LineCommentAfterSourceCode of comment: string