From a5b341ae5f0ac7e1ab0eaab7bcd40aefe0251c0a Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 1 Feb 2023 17:23:16 +0100 Subject: [PATCH 1/6] Refactor, extract closures as separate functions --- src/Compiler/Checking/CheckFormatStrings.fs | 705 ++++++++++---------- 1 file changed, 346 insertions(+), 359 deletions(-) diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index 8d287145dcb..1d71991ea76 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -3,7 +3,7 @@ module internal FSharp.Compiler.CheckFormatStrings open System.Text -open Internal.Utilities.Library +open Internal.Utilities.Library open Internal.Utilities.Library.Extras open FSharp.Compiler.ConstraintSolver open FSharp.Compiler.NameResolution @@ -14,35 +14,35 @@ open FSharp.Compiler.TypedTree open FSharp.Compiler.TypedTreeOps open FSharp.Compiler.TcGlobals -type FormatItem = Simple of TType | FuncAndVal +type FormatItem = Simple of TType | FuncAndVal -let copyAndFixupFormatTypar g m tp = +let copyAndFixupFormatTypar g m tp = let _,_,tinst = FreshenAndFixupTypars g m TyparRigidity.Flexible [] [] [tp] List.head tinst let lowestDefaultPriority = 0 (* See comment on TyparConstraint.DefaultsTo *) -let mkFlexibleFormatTypar g m tys dfltTy = +let mkFlexibleFormatTypar g m tys dfltTy = let tp = Construct.NewTypar (TyparKind.Type, TyparRigidity.Rigid, SynTypar(mkSynId m "fmt",TyparStaticReq.HeadType,true),false,TyparDynamicReq.Yes,[],false,false) tp.SetConstraints [ TyparConstraint.SimpleChoice (tys,m); TyparConstraint.DefaultsTo (lowestDefaultPriority,dfltTy,m)] copyAndFixupFormatTypar g m tp -let mkFlexibleIntFormatTypar (g: TcGlobals) m = +let mkFlexibleIntFormatTypar (g: TcGlobals) m = mkFlexibleFormatTypar g m [ g.byte_ty; g.int16_ty; g.int32_ty; g.int64_ty; g.sbyte_ty; g.uint16_ty; g.uint32_ty; g.uint64_ty;g.nativeint_ty;g.unativeint_ty; ] g.int_ty let mkFlexibleDecimalFormatTypar (g: TcGlobals) m = mkFlexibleFormatTypar g m [ g.decimal_ty ] g.decimal_ty - -let mkFlexibleFloatFormatTypar (g: TcGlobals) m = + +let mkFlexibleFloatFormatTypar (g: TcGlobals) m = mkFlexibleFormatTypar g m [ g.float_ty; g.float32_ty; g.decimal_ty ] g.float_ty -type FormatInfoRegister = - { mutable leftJustify : bool +type FormatInfoRegister = + { mutable leftJustify : bool mutable numPrefixIfPos : char option mutable addZeros : bool mutable precision : bool} -let newInfo () = +let newInfo () = { leftJustify = false numPrefixIfPos = None addZeros = false @@ -55,6 +55,155 @@ let escapeDotnetFormatString str = |> Seq.collect (fun x -> if x = '{' || x = '}' then [x;x] else [x]) |> System.String.Concat +let makeFmts (context: FormatStringCheckContext) (isInterpolated: bool) (fragRanges: range list) (fmt: string)= + let numFrags = fragRanges.Length + let sourceText = context.SourceText + let lineStartPositions = context.LineStartPositions + let length = sourceText.Length + [ for i, fragRange in List.indexed fragRanges do + let m = fragRange + if m.StartLine - 1 < lineStartPositions.Length && m.EndLine - 1 < lineStartPositions.Length then + let startIndex = lineStartPositions[m.StartLine-1] + m.StartColumn + let endIndex = lineStartPositions[m.EndLine-1] + m.EndColumn + // Note, some extra """ text may be included at end of these snippets, meaning CheckFormatString in the IDE + // may be using a slightly false format string to colorize the %d markers. This doesn't matter as there + // won't be relevant %d in these sections + // + // However we make an effort to remove these to keep the calls to GetSubStringText valid. So + // we work out how much extra text there is at the end of the last line of the fragment, + // which may or may not be quote markers. If there's no flex, we don't trim the quote marks + let endNextLineIndex = if m.EndLine < lineStartPositions.Length then lineStartPositions[m.EndLine] else endIndex + let endIndexFlex = endNextLineIndex - endIndex + let mLength = endIndex - startIndex + + if isInterpolated && i=0 && startIndex < length-4 && sourceText.SubTextEquals("$\"\"\"", startIndex) then + // Take of the ending triple quote or '{' + let fragLength = mLength - 4 - min endIndexFlex (if i = numFrags-1 then 3 else 1) + (4, sourceText.GetSubTextString(startIndex + 4, fragLength), m) + elif not isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("\"\"\"", startIndex) then + // Take of the ending triple quote or '{' + let fragLength = mLength - 2 - min endIndexFlex (if i = numFrags-1 then 3 else 1) + (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) + elif isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("$@\"", startIndex) then + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 3 - min endIndexFlex 1 + (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) + elif isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("@$\"", startIndex) then + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 3 - min endIndexFlex 1 + (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) + elif not isInterpolated && i=0 && startIndex < length-2 && sourceText.SubTextEquals("@\"", startIndex) then + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 2 - min endIndexFlex 1 + (2, sourceText.GetSubTextString(startIndex + 2, fragLength), m) + elif isInterpolated && i=0 && startIndex < length-2 && sourceText.SubTextEquals("$\"", startIndex) then + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 2 - min endIndexFlex 1 + (2, sourceText.GetSubTextString(startIndex + 2, fragLength), m) + elif isInterpolated && i <> 0 && startIndex < length-1 && sourceText.SubTextEquals("}", startIndex) then + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 1 - min endIndexFlex 1 + (1, sourceText.GetSubTextString(startIndex + 1, fragLength), m) + else + // Take of the ending quote or '{', always length 1 + let fragLength = mLength - 1 - min endIndexFlex 1 + (1, sourceText.GetSubTextString(startIndex + 1, fragLength), m) + else (1, fmt, m) ] + +module internal Parsing = + + let flags (info: FormatInfoRegister) (fmt: string) (fmtPos: int) = + let len = fmt.Length + let rec go pos = + if pos >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) + match fmt[pos] with + | '-' -> + if info.leftJustify then failwith (FSComp.SR.forFlagSetTwice("-")) + info.leftJustify <- true + go (pos+1) + | '+' -> + if info.numPrefixIfPos <> None then failwith (FSComp.SR.forPrefixFlagSpacePlusSetTwice()) + info.numPrefixIfPos <- Some '+' + go (pos+1) + | '0' -> + if info.addZeros then failwith (FSComp.SR.forFlagSetTwice("0")) + info.addZeros <- true + go (pos+1) + | ' ' -> + if info.numPrefixIfPos <> None then failwith (FSComp.SR.forPrefixFlagSpacePlusSetTwice()) + info.numPrefixIfPos <- Some ' ' + go (pos+1) + | '#' -> failwith (FSComp.SR.forHashSpecifierIsInvalid()) + | _ -> pos + go fmtPos + + let rec digitsPrecision (fmt: string) (fmtPos: int) = + if fmtPos >= fmt.Length then failwith (FSComp.SR.forBadPrecision()) + match fmt[fmtPos] with + | c when System.Char.IsDigit c -> digitsPrecision fmt (fmtPos+1) + | _ -> fmtPos + + let precision (info: FormatInfoRegister) (fmt: string) (fmtPos: int) = + if fmtPos >= fmt.Length then failwith (FSComp.SR.forBadWidth()) + match fmt[fmtPos] with + | c when System.Char.IsDigit c -> info.precision <- true; false,digitsPrecision fmt (fmtPos+1) + | '*' -> info.precision <- true; true,(fmtPos+1) + | _ -> failwith (FSComp.SR.forPrecisionMissingAfterDot()) + + let optionalDotAndPrecision (info: FormatInfoRegister) (fmt: string) (fmtPos: int) = + if fmtPos >= fmt.Length then failwith (FSComp.SR.forBadPrecision()) + match fmt[fmtPos] with + | '.' -> precision info fmt (fmtPos+1) + | _ -> false,fmtPos + + let rec digitsWidthAndPrecision (info: FormatInfoRegister) (fmt: string) (fmtPos: int) (intAcc: int) = + let len = fmt.Length + let rec go pos n = + if pos >= len then failwith (FSComp.SR.forBadPrecision()) + match fmt[pos] with + | c when System.Char.IsDigit c -> go (pos+1) (n*10 + int c - int '0') + | _ -> Some n, optionalDotAndPrecision info fmt pos + go fmtPos intAcc + + let widthAndPrecision (info: FormatInfoRegister) (fmt: string) (fmtPos: int) = + if fmtPos >= fmt.Length then failwith (FSComp.SR.forBadPrecision()) + match fmt[fmtPos] with + | c when System.Char.IsDigit c -> false,digitsWidthAndPrecision info fmt fmtPos 0 + | '*' -> true, (None, optionalDotAndPrecision info fmt (fmtPos+1)) + | _ -> false, (None, optionalDotAndPrecision info fmt fmtPos) + + let position (fmt: string) (fmtPos: int) = + let len = fmt.Length + + let rec digitsPosition n pos = + if pos >= len then failwith (FSComp.SR.forBadPrecision()) + match fmt[pos] with + | c when System.Char.IsDigit c -> digitsPosition (n*10 + int c - int '0') (pos+1) + | '$' -> Some n, pos+1 + | _ -> None, pos + + match fmt[fmtPos] with + | c when c >= '1' && c <= '9' -> + let p, pos' = digitsPosition (int c - int '0') (fmtPos+1) + if p = None then None, fmtPos else p, pos' + | _ -> None, fmtPos + + // Explicitly typed holes in interpolated strings "....%d{x}..." get additional '%P()' as a hole place marker + let skipPossibleInterpolationHole isInterpolated isFormattableString (fmt: string) i = + let len = fmt.Length + if isInterpolated then + if i+1 < len && fmt[i] = '%' && fmt[i+1] = 'P' then + let i = i + 2 + if i+1 < len && fmt[i] = '(' && fmt[i+1] = ')' then + if isFormattableString then + failwith (FSComp.SR.forFormatInvalidForInterpolated4()) + i + 2 + else + failwith (FSComp.SR.forFormatInvalidForInterpolated2()) + else + failwith (FSComp.SR.forFormatInvalidForInterpolated()) + else i + let parseFormatStringInternal (m: range) (fragRanges: range list) @@ -64,7 +213,7 @@ let parseFormatStringInternal (context: FormatStringCheckContext option) (fmt: string) printerArgTy - printerResidueTy = + printerResidueTy = // As background: the F# compiler tokenizes strings on the assumption that the only thing you need from // them is the actual corresponding text, e.g. of a string literal. This means many different textual input strings @@ -81,7 +230,7 @@ let parseFormatStringInternal // source of the string by going back and getting the format string from the input source file by using the // relevant ranges // - // For interpolated strings this may involve many fragments, e.g. + // For interpolated strings this may involve many fragments, e.g. // $"abc %d{" // "} def %s{" // "} xyz" @@ -95,93 +244,22 @@ let parseFormatStringInternal // let escapeFormatStringEnabled = g.langVersion.SupportsFeature Features.LanguageFeature.EscapeDotnetFormattableStrings - let fmt, fragments = - - //printfn "--------------------" - //printfn "context.IsSome = %b" context.IsSome - //printfn "fmt = <<<%s>>>" fmt - //printfn "isInterpolated = %b" isInterpolated - //printfn "fragRanges = %A" fragRanges - + let fmt, fragments = match context with | Some context when fragRanges.Length > 0 -> - let sourceText = context.SourceText - //printfn "sourceText.IsSome = %b" sourceText.IsSome - let lineStartPositions = context.LineStartPositions - //printfn "lineStartPositions.Length = %d" lineStartPositions.Length - let length = sourceText.Length - let numFrags = fragRanges.Length - let fmts = - [ for i, fragRange in List.indexed fragRanges do - let m = fragRange - //printfn "m.EndLine = %d" m.EndLine - if m.StartLine - 1 < lineStartPositions.Length && m.EndLine - 1 < lineStartPositions.Length then - let startIndex = lineStartPositions[m.StartLine-1] + m.StartColumn - let endIndex = lineStartPositions[m.EndLine-1] + m.EndColumn - // Note, some extra """ text may be included at end of these snippets, meaning CheckFormatString in the IDE - // may be using a slightly false format string to colorize the %d markers. This doesn't matter as there - // won't be relevant %d in these sections - // - // However we make an effort to remove these to keep the calls to GetSubStringText valid. So - // we work out how much extra text there is at the end of the last line of the fragment, - // which may or may not be quote markers. If there's no flex, we don't trim the quote marks - let endNextLineIndex = if m.EndLine < lineStartPositions.Length then lineStartPositions[m.EndLine] else endIndex - let endIndexFlex = endNextLineIndex - endIndex - let mLength = endIndex - startIndex - - //let startIndex2 = if m.StartLine < lineStartPositions.Length then lineStartPositions.[m.StartLine] else startIndex - //let sourceLineFromOffset = sourceText.GetSubTextString(startIndex, (startIndex2 - startIndex)) - //printfn "i = %d, mLength = %d, endIndexFlex = %d, sourceLineFromOffset = <<<%s>>>" i mLength endIndexFlex sourceLineFromOffset - - if isInterpolated && i=0 && startIndex < length-4 && sourceText.SubTextEquals("$\"\"\"", startIndex) then - // Take of the ending triple quote or '{' - let fragLength = mLength - 4 - min endIndexFlex (if i = numFrags-1 then 3 else 1) - (4, sourceText.GetSubTextString(startIndex + 4, fragLength), m) - elif not isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("\"\"\"", startIndex) then - // Take of the ending triple quote or '{' - let fragLength = mLength - 2 - min endIndexFlex (if i = numFrags-1 then 3 else 1) - (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) - elif isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("$@\"", startIndex) then - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 3 - min endIndexFlex 1 - (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) - elif isInterpolated && i=0 && startIndex < length-3 && sourceText.SubTextEquals("@$\"", startIndex) then - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 3 - min endIndexFlex 1 - (3, sourceText.GetSubTextString(startIndex + 3, fragLength), m) - elif not isInterpolated && i=0 && startIndex < length-2 && sourceText.SubTextEquals("@\"", startIndex) then - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 2 - min endIndexFlex 1 - (2, sourceText.GetSubTextString(startIndex + 2, fragLength), m) - elif isInterpolated && i=0 && startIndex < length-2 && sourceText.SubTextEquals("$\"", startIndex) then - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 2 - min endIndexFlex 1 - (2, sourceText.GetSubTextString(startIndex + 2, fragLength), m) - elif isInterpolated && i <> 0 && startIndex < length-1 && sourceText.SubTextEquals("}", startIndex) then - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 1 - min endIndexFlex 1 - (1, sourceText.GetSubTextString(startIndex + 1, fragLength), m) - else - // Take of the ending quote or '{', always length 1 - let fragLength = mLength - 1 - min endIndexFlex 1 - (1, sourceText.GetSubTextString(startIndex + 1, fragLength), m) - else (1, fmt, m) ] - - //printfn "fmts = %A" fmts + let fmts = makeFmts context isInterpolated fragRanges fmt // Join the fragments with holes. Note this join is only used on the IDE path, // the CheckExpressions.fs does its own joining with the right alignments etc. substituted // On the IDE path we don't do any checking of these in this file (some checking is - // done in CheckExpressions.fs) so it's ok to join with just '%P()'. - let fmt = fmts |> List.map p23 |> String.concat "%P()" - let fragments, _ = + // done in CheckExpressions.fs) so it's ok to join with just '%P()'. + let fmt = fmts |> List.map p23 |> String.concat "%P()" + let fragments, _ = (0, fmts) ||> List.mapFold (fun i (offset, fmt, fragRange) -> (i, offset, fragRange), i + fmt.Length + 4) // the '4' is the length of '%P()' joins - //printfn "fmt2 = <<<%s>>>" fmt - //printfn "fragments = %A" fragments fmt, fragments - | _ -> + | _ -> // Don't muck with the fmt when there is no source code context to go get the original // source code (i.e. when compiling or background checking) (if escapeFormatStringEnabled then escapeDotnetFormatString fmt else fmt), [ (0, 1, m) ] @@ -193,307 +271,216 @@ let parseFormatStringInternal // For FormattableString we collect a .NET Format String with {0} etc. replacing text. '%%' are replaced // by '%', we check there are no '%' formats, and '{{' and '}}' are *not* replaced since the subsequent // call to String.Format etc. will process them. - let dotnetFormatString = StringBuilder() - let appendToDotnetFormatString (s: string) = dotnetFormatString.Append(s) |> ignore + let dotnetFormatString = StringBuilder() + let appendToDotnetFormatString (s: string) = dotnetFormatString.Append(s) |> ignore let mutable dotnetFormatStringInterpolationHoleCount = 0 let percentATys = ResizeArray<_>() // fragLine, fragCol - track our location w.r.t. the marker for the start of this chunk - // - let rec parseLoop acc (i, fragLine, fragCol) fragments = - + // + let rec parseLoop acc (i, fragLine, fragCol) fragments = + // Check if we've moved into the next fragment. Note this will always activate on // the first step, i.e. when i=0 let struct (fragLine, fragCol, fragments) = - match fragments with + match fragments with | (idx, fragOffset, fragRange: range)::rest when i >= idx -> - //printfn "i = %d, idx = %d, moving into next fragment at %A plus fragOffset %d" i idx fragRange fragOffset struct (fragRange.StartLine, fragRange.StartColumn + fragOffset, rest) | _ -> struct (fragLine, fragCol, fragments) - //printfn "parseLoop: i = %d, fragLine = %d, fragCol = %d" i fragLine fragCol if i >= len then let argTys = if acc |> List.forall (fun (p, _) -> p = None) then // without positional specifiers acc |> List.map snd |> List.rev - else + else failwith (FSComp.SR.forPositionalSpecifiersNotPermitted()) argTys - elif System.Char.IsSurrogatePair(fmt,i) then + elif System.Char.IsSurrogatePair(fmt,i) then appendToDotnetFormatString fmt[i..i+1] parseLoop acc (i+2, fragLine, fragCol+2) fragments - else + else let c = fmt[i] match c with - | '%' -> - let startFragCol = fragCol - let fragCol = fragCol+1 - let i = i+1 - if i >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) - let info = newInfo() - - let rec flags i = - if i >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) - match fmt[i] with - | '-' -> - if info.leftJustify then failwith (FSComp.SR.forFlagSetTwice("-")) - info.leftJustify <- true - flags(i+1) - | '+' -> - if info.numPrefixIfPos <> None then failwith (FSComp.SR.forPrefixFlagSpacePlusSetTwice()) - info.numPrefixIfPos <- Some '+' - flags(i+1) - | '0' -> - if info.addZeros then failwith (FSComp.SR.forFlagSetTwice("0")) - info.addZeros <- true - flags(i+1) - | ' ' -> - if info.numPrefixIfPos <> None then failwith (FSComp.SR.forPrefixFlagSpacePlusSetTwice()) - info.numPrefixIfPos <- Some ' ' - flags(i+1) - | '#' -> failwith (FSComp.SR.forHashSpecifierIsInvalid()) - | _ -> i - - let rec digitsPrecision i = - if i >= len then failwith (FSComp.SR.forBadPrecision()) - match fmt[i] with - | c when System.Char.IsDigit c -> digitsPrecision (i+1) - | _ -> i - - let precision i = - if i >= len then failwith (FSComp.SR.forBadWidth()) - match fmt[i] with - | c when System.Char.IsDigit c -> info.precision <- true; false,digitsPrecision (i+1) - | '*' -> info.precision <- true; true,(i+1) - | _ -> failwith (FSComp.SR.forPrecisionMissingAfterDot()) - - let optionalDotAndPrecision i = - if i >= len then failwith (FSComp.SR.forBadPrecision()) - match fmt[i] with - | '.' -> precision (i+1) - | _ -> false,i - - let rec digitsWidthAndPrecision n i = - if i >= len then failwith (FSComp.SR.forBadPrecision()) - match fmt[i] with - | c when System.Char.IsDigit c -> digitsWidthAndPrecision (n*10 + int c - int '0') (i+1) - | _ -> Some n, optionalDotAndPrecision i - - let widthAndPrecision i = - if i >= len then failwith (FSComp.SR.forBadPrecision()) - match fmt[i] with - | c when System.Char.IsDigit c -> false,digitsWidthAndPrecision 0 i - | '*' -> true, (None, optionalDotAndPrecision (i+1)) - | _ -> false, (None, optionalDotAndPrecision i) - - let rec digitsPosition n i = - if i >= len then failwith (FSComp.SR.forBadPrecision()) - match fmt[i] with - | c when System.Char.IsDigit c -> digitsPosition (n*10 + int c - int '0') (i+1) - | '$' -> Some n, i+1 - | _ -> None, i - - let position i = - match fmt[i] with - | c when c >= '1' && c <= '9' -> - let p, i' = digitsPosition (int c - int '0') (i+1) - if p = None then None, i else p, i' - | _ -> None, i - - let oldI = i - let posi, i = position i - let fragCol = fragCol + i - oldI - - let oldI = i - let i = flags i - let fragCol = fragCol + i - oldI - - let oldI = i - let widthArg,(widthValue, (precisionArg,i)) = widthAndPrecision i - let fragCol = fragCol + i - oldI - - if i >= len then failwith (FSComp.SR.forBadPrecision()) - - let acc = if precisionArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc - - let acc = if widthArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc - - let checkNoPrecision c = - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(c.ToString())) - - let checkNoZeroFlag c = - if info.addZeros then failwith (FSComp.SR.forDoesNotSupportZeroFlag(c.ToString())) - - let checkNoNumericPrefix c = - match info.numPrefixIfPos with - | Some n -> failwith (FSComp.SR.forDoesNotSupportPrefixFlag(c.ToString(), n.ToString())) - | None -> () - - let checkOtherFlags c = - checkNoPrecision c - checkNoZeroFlag c - checkNoNumericPrefix c - - // Explicitly typed holes in interpolated strings "....%d{x}..." get additional '%P()' as a hole place marker - let skipPossibleInterpolationHole i = - if isInterpolated then - if i+1 < len && fmt[i] = '%' && fmt[i+1] = 'P' then - let i = i + 2 - if i+1 < len && fmt[i] = '(' && fmt[i+1] = ')' then - if isFormattableString then - failwith (FSComp.SR.forFormatInvalidForInterpolated4()) - i + 2 - else - failwith (FSComp.SR.forFormatInvalidForInterpolated2()) - else - failwith (FSComp.SR.forFormatInvalidForInterpolated()) - else i - - // Implicitly typed holes in interpolated strings are translated to '... %P(...)...' in the - // type checker. They should always have '(...)' after for format string. - let requireAndSkipInterpolationHoleFormat i = - if i < len && fmt[i] = '(' then - let i2 = fmt.IndexOf(")", i+1) - if i2 = -1 then - failwith (FSComp.SR.forFormatInvalidForInterpolated3()) - else - let dotnetAlignment = match widthValue with None -> "" | Some w -> "," + (if info.leftJustify then "-" else "") + string w - let dotnetNumberFormat = match fmt[i+1..i2-1] with "" -> "" | s -> ":" + s - appendToDotnetFormatString ("{" + string dotnetFormatStringInterpolationHoleCount + dotnetAlignment + dotnetNumberFormat + "}") - dotnetFormatStringInterpolationHoleCount <- dotnetFormatStringInterpolationHoleCount + 1 - i2+1 - else - failwith (FSComp.SR.forFormatInvalidForInterpolated3()) - - let collectSpecifierLocation fragLine fragCol numStdArgs = - match context with - | Some _ -> - let numArgsForSpecifier = - numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) - specifierLocations.Add( - (Range.mkFileIndexRange m.FileIndex - (Position.mkPos fragLine startFragCol) - (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) - | None -> () - - let ch = fmt[i] - match ch with - | '%' -> - collectSpecifierLocation fragLine fragCol 0 - appendToDotnetFormatString "%" - parseLoop acc (i+1, fragLine, fragCol+1) fragments - - | 'd' | 'i' | 'u' | 'B' | 'o' | 'x' | 'X' -> - if ch = 'B' then DiagnosticsLogger.checkLanguageFeatureError g.langVersion Features.LanguageFeature.PrintfBinaryFormat m - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'l' | 'L' -> - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) - let fragCol = fragCol+1 - let i = i+1 - - // "bad format specifier ... In F# code you can use %d, %x, %o or %u instead ..." - if i >= len then - failwith (FSComp.SR.forBadFormatSpecifier()) - // Always error for %l and %Lx - failwith (FSComp.SR.forLIsUnnecessary()) - match fmt[i] with - | 'd' | 'i' | 'o' | 'u' | 'x' | 'X' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - | _ -> failwith (FSComp.SR.forBadFormatSpecifier()) - - | 'h' | 'H' -> - failwith (FSComp.SR.forHIsUnnecessary()) - - | 'M' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleDecimalFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'f' | 'F' | 'e' | 'E' | 'g' | 'G' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleFloatFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'b' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.bool_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 'c' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.char_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 's' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.string_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 'O' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, fragCol+1) fragments - - // residue of hole "...{n}..." in interpolated strings become %P(...) - | 'P' when isInterpolated -> - checkOtherFlags ch - let i = requireAndSkipInterpolationHoleFormat (i+1) - // Note, the fragCol doesn't advance at all as these are magically inserted. - parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, startFragCol) fragments - - | 'A' -> - if g.useReflectionFreeCodeGen then - failwith (FSComp.SR.forPercentAInReflectionFreeCode()) - - match info.numPrefixIfPos with - | None // %A has BindingFlags=Public, %+A has BindingFlags=Public | NonPublic - | Some '+' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - let aTy = NewInferenceType g - percentATys.Add(aTy) - parseLoop ((posi, aTy) :: acc) (i, fragLine, fragCol+1) fragments - | Some n -> - failwith (FSComp.SR.forDoesNotSupportPrefixFlag(ch.ToString(), n.ToString())) - - | 'a' -> - checkOtherFlags ch - let aTy = NewInferenceType g - let fTy = mkFunTy g printerArgTy (mkFunTy g aTy printerResidueTy) - collectSpecifierLocation fragLine fragCol 2 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((Option.map ((+)1) posi, aTy) :: (posi, fTy) :: acc) (i, fragLine, fragCol+1) fragments - - | 't' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFunTy g printerArgTy printerResidueTy) :: acc) (i, fragLine, fragCol+1) fragments - - | c -> failwith (FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)) - + | '%' -> parseSpecifier acc (i, fragLine, fragCol) fragments | '\n' -> appendToDotnetFormatString fmt[i..i] parseLoop acc (i+1, fragLine+1, 0) fragments | _ -> appendToDotnetFormatString fmt[i..i] parseLoop acc (i+1, fragLine, fragCol+1) fragments - + + and parseSpecifier acc (i, fragLine, fragCol) fragments = + let startFragCol = fragCol + let fragCol = fragCol+1 + let i = i+1 + if i >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) + let info = newInfo() + + let oldI = i + let posi, i = Parsing.position fmt i + let fragCol = fragCol + i - oldI + + let oldI = i + let i = Parsing.flags info fmt i + let fragCol = fragCol + i - oldI + + let oldI = i + let widthArg,(widthValue, (precisionArg,i)) = Parsing.widthAndPrecision info fmt i + let fragCol = fragCol + i - oldI + + if i >= len then failwith (FSComp.SR.forBadPrecision()) + + let acc = if precisionArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc + + let acc = if widthArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc + + let checkOtherFlags c = + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(c.ToString())) + if info.addZeros then failwith (FSComp.SR.forDoesNotSupportZeroFlag(c.ToString())) + match info.numPrefixIfPos with + | Some n -> failwith (FSComp.SR.forDoesNotSupportPrefixFlag(c.ToString(), n.ToString())) + | None -> () + + let skipPossibleInterpolationHole pos = Parsing.skipPossibleInterpolationHole isInterpolated isFormattableString fmt pos + + // Implicitly typed holes in interpolated strings are translated to '... %P(...)...' in the + // type checker. They should always have '(...)' after for format string. + let requireAndSkipInterpolationHoleFormat i = + if i < len && fmt[i] = '(' then + let i2 = fmt.IndexOf(")", i+1) + if i2 = -1 then + failwith (FSComp.SR.forFormatInvalidForInterpolated3()) + else + let dotnetAlignment = match widthValue with None -> "" | Some w -> "," + (if info.leftJustify then "-" else "") + string w + let dotnetNumberFormat = match fmt[i+1..i2-1] with "" -> "" | s -> ":" + s + appendToDotnetFormatString ("{" + string dotnetFormatStringInterpolationHoleCount + dotnetAlignment + dotnetNumberFormat + "}") + dotnetFormatStringInterpolationHoleCount <- dotnetFormatStringInterpolationHoleCount + 1 + i2+1 + else + failwith (FSComp.SR.forFormatInvalidForInterpolated3()) + + let collectSpecifierLocation fragLine fragCol numStdArgs = + match context with + | Some _ -> + let numArgsForSpecifier = + numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine startFragCol) + (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) + | None -> () + + let ch = fmt[i] + match ch with + | '%' -> + collectSpecifierLocation fragLine fragCol 0 + appendToDotnetFormatString "%" + parseLoop acc (i+1, fragLine, fragCol+1) fragments + + | 'd' | 'i' | 'u' | 'B' | 'o' | 'x' | 'X' -> + if ch = 'B' then DiagnosticsLogger.checkLanguageFeatureError g.langVersion Features.LanguageFeature.PrintfBinaryFormat m + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'l' | 'L' -> + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) + let fragCol = fragCol+1 + let i = i+1 + + // "bad format specifier ... In F# code you can use %d, %x, %o or %u instead ..." + if i >= len then + failwith (FSComp.SR.forBadFormatSpecifier()) + // Always error for %l and %Lx + failwith (FSComp.SR.forLIsUnnecessary()) + match fmt[i] with + | 'd' | 'i' | 'o' | 'u' | 'x' | 'X' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + | _ -> failwith (FSComp.SR.forBadFormatSpecifier()) + + | 'h' | 'H' -> + failwith (FSComp.SR.forHIsUnnecessary()) + + | 'M' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleDecimalFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'f' | 'F' | 'e' | 'E' | 'g' | 'G' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleFloatFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'b' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.bool_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 'c' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.char_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 's' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.string_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 'O' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, fragCol+1) fragments + + // residue of hole "...{n}..." in interpolated strings become %P(...) + | 'P' when isInterpolated -> + checkOtherFlags ch + let i = requireAndSkipInterpolationHoleFormat (i+1) + // Note, the fragCol doesn't advance at all as these are magically inserted. + parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, startFragCol) fragments + + | 'A' -> + if g.useReflectionFreeCodeGen then + failwith (FSComp.SR.forPercentAInReflectionFreeCode()) + + match info.numPrefixIfPos with + | None // %A has BindingFlags=Public, %+A has BindingFlags=Public | NonPublic + | Some '+' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + let aTy = NewInferenceType g + percentATys.Add(aTy) + parseLoop ((posi, aTy) :: acc) (i, fragLine, fragCol+1) fragments + | Some n -> + failwith (FSComp.SR.forDoesNotSupportPrefixFlag(ch.ToString(), n.ToString())) + + | 'a' -> + checkOtherFlags ch + let aTy = NewInferenceType g + let fTy = mkFunTy g printerArgTy (mkFunTy g aTy printerResidueTy) + collectSpecifierLocation fragLine fragCol 2 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((Option.map ((+)1) posi, aTy) :: (posi, fTy) :: acc) (i, fragLine, fragCol+1) fragments + + | 't' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFunTy g printerArgTy printerResidueTy) :: acc) (i, fragLine, fragCol+1) fragments + + | c -> failwith (FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)) + let results = parseLoop [] (0, 0, m.StartColumn) fragments results, Seq.toList specifierLocations, dotnetFormatString.ToString(), percentATys.ToArray() -let ParseFormatString m fragmentRanges g isInterpolated isFormattableString formatStringCheckContext fmt printerArgTy printerResidueTy printerResultTy = +let ParseFormatString m fragmentRanges g isInterpolated isFormattableString formatStringCheckContext fmt printerArgTy printerResidueTy printerResultTy = let argTys, specifierLocations, dotnetFormatString, percentATys = parseFormatStringInternal m fragmentRanges g isInterpolated isFormattableString formatStringCheckContext fmt printerArgTy printerResidueTy let printerTy = List.foldBack (mkFunTy g) argTys printerResultTy From a8682c18cf1bc6e267acfd628b2094a09928651d Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 2 Feb 2023 16:05:31 +0100 Subject: [PATCH 2/6] Add unit test --- .../Language/InterpolatedStringsTests.fs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs index c51836e105a..0347e982900 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs @@ -48,3 +48,12 @@ printf $"{a.Format}" Assert.Equal("%", $"%%") Assert.Equal("42%", $"{42}%%") Assert.Equal("% 42", $"%%%3d{42}") + + [] + let ``Percent signs separated by format specifier's flags`` () = + Fsx """ +let s = $"...%-%...{0}" + """ + |> compile + |> shouldFail + |> withSingleDiagnostic (Error 3376, Line 2, Col 9, Line 2, Col 24, "Invalid interpolated string. Bad format specifier: '%'") \ No newline at end of file From 4229b4b5dd780ee954c62939bcac732fa629ef34 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 2 Feb 2023 16:17:41 +0100 Subject: [PATCH 3/6] Fix the issue with format specifiers Change parsing format specifiers so that it first checks whether it sees escaped `%` before it starts parsing flags/width/precision. --- src/Compiler/Checking/CheckFormatStrings.fs | 304 ++++++++++---------- 1 file changed, 155 insertions(+), 149 deletions(-) diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index 1d71991ea76..29c3b88e0b4 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -313,169 +313,175 @@ let parseFormatStringInternal and parseSpecifier acc (i, fragLine, fragCol) fragments = let startFragCol = fragCol let fragCol = fragCol+1 - let i = i+1 - if i >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) - let info = newInfo() - - let oldI = i - let posi, i = Parsing.position fmt i - let fragCol = fragCol + i - oldI - - let oldI = i - let i = Parsing.flags info fmt i - let fragCol = fragCol + i - oldI - - let oldI = i - let widthArg,(widthValue, (precisionArg,i)) = Parsing.widthAndPrecision info fmt i - let fragCol = fragCol + i - oldI - - if i >= len then failwith (FSComp.SR.forBadPrecision()) - - let acc = if precisionArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc - - let acc = if widthArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc - - let checkOtherFlags c = - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(c.ToString())) - if info.addZeros then failwith (FSComp.SR.forDoesNotSupportZeroFlag(c.ToString())) - match info.numPrefixIfPos with - | Some n -> failwith (FSComp.SR.forDoesNotSupportPrefixFlag(c.ToString(), n.ToString())) - | None -> () - - let skipPossibleInterpolationHole pos = Parsing.skipPossibleInterpolationHole isInterpolated isFormattableString fmt pos - - // Implicitly typed holes in interpolated strings are translated to '... %P(...)...' in the - // type checker. They should always have '(...)' after for format string. - let requireAndSkipInterpolationHoleFormat i = - if i < len && fmt[i] = '(' then - let i2 = fmt.IndexOf(")", i+1) - if i2 = -1 then - failwith (FSComp.SR.forFormatInvalidForInterpolated3()) - else - let dotnetAlignment = match widthValue with None -> "" | Some w -> "," + (if info.leftJustify then "-" else "") + string w - let dotnetNumberFormat = match fmt[i+1..i2-1] with "" -> "" | s -> ":" + s - appendToDotnetFormatString ("{" + string dotnetFormatStringInterpolationHoleCount + dotnetAlignment + dotnetNumberFormat + "}") - dotnetFormatStringInterpolationHoleCount <- dotnetFormatStringInterpolationHoleCount + 1 - i2+1 - else - failwith (FSComp.SR.forFormatInvalidForInterpolated3()) - - let collectSpecifierLocation fragLine fragCol numStdArgs = + if fmt[i..(i+1)] = "%%" then match context with | Some _ -> - let numArgsForSpecifier = - numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) specifierLocations.Add( (Range.mkFileIndexRange m.FileIndex (Position.mkPos fragLine startFragCol) - (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) + (Position.mkPos fragLine (fragCol + 1))), 0) | None -> () - - let ch = fmt[i] - match ch with - | '%' -> - collectSpecifierLocation fragLine fragCol 0 appendToDotnetFormatString "%" - parseLoop acc (i+1, fragLine, fragCol+1) fragments - - | 'd' | 'i' | 'u' | 'B' | 'o' | 'x' | 'X' -> - if ch = 'B' then DiagnosticsLogger.checkLanguageFeatureError g.langVersion Features.LanguageFeature.PrintfBinaryFormat m - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'l' | 'L' -> - if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) - let fragCol = fragCol+1 + parseLoop acc (i+2, fragLine, fragCol+1) fragments + else let i = i+1 + if i >= len then failwith (FSComp.SR.forMissingFormatSpecifier()) + let info = newInfo() + + let oldI = i + let posi, i = Parsing.position fmt i + let fragCol = fragCol + i - oldI + + let oldI = i + let i = Parsing.flags info fmt i + let fragCol = fragCol + i - oldI + + let oldI = i + let widthArg,(widthValue, (precisionArg,i)) = Parsing.widthAndPrecision info fmt i + let fragCol = fragCol + i - oldI + + if i >= len then failwith (FSComp.SR.forBadPrecision()) + + let acc = if precisionArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc + + let acc = if widthArg then (Option.map ((+)1) posi, g.int_ty) :: acc else acc + + let checkOtherFlags c = + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(c.ToString())) + if info.addZeros then failwith (FSComp.SR.forDoesNotSupportZeroFlag(c.ToString())) + match info.numPrefixIfPos with + | Some n -> failwith (FSComp.SR.forDoesNotSupportPrefixFlag(c.ToString(), n.ToString())) + | None -> () + + let skipPossibleInterpolationHole pos = Parsing.skipPossibleInterpolationHole isInterpolated isFormattableString fmt pos + + // Implicitly typed holes in interpolated strings are translated to '... %P(...)...' in the + // type checker. They should always have '(...)' after for format string. + let requireAndSkipInterpolationHoleFormat i = + if i < len && fmt[i] = '(' then + let i2 = fmt.IndexOf(")", i+1) + if i2 = -1 then + failwith (FSComp.SR.forFormatInvalidForInterpolated3()) + else + let dotnetAlignment = match widthValue with None -> "" | Some w -> "," + (if info.leftJustify then "-" else "") + string w + let dotnetNumberFormat = match fmt[i+1..i2-1] with "" -> "" | s -> ":" + s + appendToDotnetFormatString ("{" + string dotnetFormatStringInterpolationHoleCount + dotnetAlignment + dotnetNumberFormat + "}") + dotnetFormatStringInterpolationHoleCount <- dotnetFormatStringInterpolationHoleCount + 1 + i2+1 + else + failwith (FSComp.SR.forFormatInvalidForInterpolated3()) - // "bad format specifier ... In F# code you can use %d, %x, %o or %u instead ..." - if i >= len then - failwith (FSComp.SR.forBadFormatSpecifier()) - // Always error for %l and %Lx - failwith (FSComp.SR.forLIsUnnecessary()) - match fmt[i] with - | 'd' | 'i' | 'o' | 'u' | 'x' | 'X' -> + let collectSpecifierLocation fragLine fragCol numStdArgs = + match context with + | Some _ -> + let numArgsForSpecifier = + numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0) + specifierLocations.Add( + (Range.mkFileIndexRange m.FileIndex + (Position.mkPos fragLine startFragCol) + (Position.mkPos fragLine (fragCol + 1))), numArgsForSpecifier) + | None -> () + + let ch = fmt[i] + match ch with + | 'd' | 'i' | 'u' | 'B' | 'o' | 'x' | 'X' -> + if ch = 'B' then DiagnosticsLogger.checkLanguageFeatureError g.langVersion Features.LanguageFeature.PrintfBinaryFormat m + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) collectSpecifierLocation fragLine fragCol 1 let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - | _ -> failwith (FSComp.SR.forBadFormatSpecifier()) - - | 'h' | 'H' -> - failwith (FSComp.SR.forHIsUnnecessary()) - - | 'M' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleDecimalFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'f' | 'F' | 'e' | 'E' | 'g' | 'G' -> - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFlexibleFloatFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments - - | 'b' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.bool_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 'c' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.char_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 's' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, g.string_ty) :: acc) (i, fragLine, fragCol+1) fragments - - | 'O' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, fragCol+1) fragments - - // residue of hole "...{n}..." in interpolated strings become %P(...) - | 'P' when isInterpolated -> - checkOtherFlags ch - let i = requireAndSkipInterpolationHoleFormat (i+1) - // Note, the fragCol doesn't advance at all as these are magically inserted. - parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, startFragCol) fragments - - | 'A' -> - if g.useReflectionFreeCodeGen then - failwith (FSComp.SR.forPercentAInReflectionFreeCode()) - - match info.numPrefixIfPos with - | None // %A has BindingFlags=Public, %+A has BindingFlags=Public | NonPublic - | Some '+' -> + parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'l' | 'L' -> + if info.precision then failwith (FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())) + let fragCol = fragCol+1 + let i = i+1 + + // "bad format specifier ... In F# code you can use %d, %x, %o or %u instead ..." + if i >= len then + failwith (FSComp.SR.forBadFormatSpecifier()) + // Always error for %l and %Lx + failwith (FSComp.SR.forLIsUnnecessary()) + match fmt[i] with + | 'd' | 'i' | 'o' | 'u' | 'x' | 'X' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + | _ -> failwith (FSComp.SR.forBadFormatSpecifier()) + + | 'h' | 'H' -> + failwith (FSComp.SR.forHIsUnnecessary()) + + | 'M' -> collectSpecifierLocation fragLine fragCol 1 let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleDecimalFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'f' | 'F' | 'e' | 'E' | 'g' | 'G' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFlexibleFloatFormatTypar g m) :: acc) (i, fragLine, fragCol+1) fragments + + | 'b' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.bool_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 'c' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.char_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 's' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, g.string_ty) :: acc) (i, fragLine, fragCol+1) fragments + + | 'O' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, fragCol+1) fragments + + // residue of hole "...{n}..." in interpolated strings become %P(...) + | 'P' when isInterpolated -> + checkOtherFlags ch + let i = requireAndSkipInterpolationHoleFormat (i+1) + // Note, the fragCol doesn't advance at all as these are magically inserted. + parseLoop ((posi, NewInferenceType g) :: acc) (i, fragLine, startFragCol) fragments + + | 'A' -> + if g.useReflectionFreeCodeGen then + failwith (FSComp.SR.forPercentAInReflectionFreeCode()) + + match info.numPrefixIfPos with + | None // %A has BindingFlags=Public, %+A has BindingFlags=Public | NonPublic + | Some '+' -> + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + let aTy = NewInferenceType g + percentATys.Add(aTy) + parseLoop ((posi, aTy) :: acc) (i, fragLine, fragCol+1) fragments + | Some n -> + failwith (FSComp.SR.forDoesNotSupportPrefixFlag(ch.ToString(), n.ToString())) + + | 'a' -> + checkOtherFlags ch let aTy = NewInferenceType g - percentATys.Add(aTy) - parseLoop ((posi, aTy) :: acc) (i, fragLine, fragCol+1) fragments - | Some n -> - failwith (FSComp.SR.forDoesNotSupportPrefixFlag(ch.ToString(), n.ToString())) - - | 'a' -> - checkOtherFlags ch - let aTy = NewInferenceType g - let fTy = mkFunTy g printerArgTy (mkFunTy g aTy printerResidueTy) - collectSpecifierLocation fragLine fragCol 2 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((Option.map ((+)1) posi, aTy) :: (posi, fTy) :: acc) (i, fragLine, fragCol+1) fragments - - | 't' -> - checkOtherFlags ch - collectSpecifierLocation fragLine fragCol 1 - let i = skipPossibleInterpolationHole (i+1) - parseLoop ((posi, mkFunTy g printerArgTy printerResidueTy) :: acc) (i, fragLine, fragCol+1) fragments - - | c -> failwith (FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)) + let fTy = mkFunTy g printerArgTy (mkFunTy g aTy printerResidueTy) + collectSpecifierLocation fragLine fragCol 2 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((Option.map ((+)1) posi, aTy) :: (posi, fTy) :: acc) (i, fragLine, fragCol+1) fragments + + | 't' -> + checkOtherFlags ch + collectSpecifierLocation fragLine fragCol 1 + let i = skipPossibleInterpolationHole (i+1) + parseLoop ((posi, mkFunTy g printerArgTy printerResidueTy) :: acc) (i, fragLine, fragCol+1) fragments + + | c -> failwith (FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)) let results = parseLoop [] (0, 0, m.StartColumn) fragments results, Seq.toList specifierLocations, dotnetFormatString.ToString(), percentATys.ToArray() From d30847704b303e5e87933439d16eee34ec9caf2d Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 8 Feb 2023 15:31:02 +0100 Subject: [PATCH 4/6] Restore previous behavior but add a warning --- src/Compiler/Checking/CheckFormatStrings.fs | 8 ++++++++ .../Language/InterpolatedStringsTests.fs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Checking/CheckFormatStrings.fs b/src/Compiler/Checking/CheckFormatStrings.fs index 29c3b88e0b4..428ac64ef61 100644 --- a/src/Compiler/Checking/CheckFormatStrings.fs +++ b/src/Compiler/Checking/CheckFormatStrings.fs @@ -6,6 +6,7 @@ open System.Text open Internal.Utilities.Library open Internal.Utilities.Library.Extras open FSharp.Compiler.ConstraintSolver +open FSharp.Compiler.DiagnosticsLogger open FSharp.Compiler.NameResolution open FSharp.Compiler.Syntax open FSharp.Compiler.SyntaxTreeOps @@ -481,6 +482,13 @@ let parseFormatStringInternal let i = skipPossibleInterpolationHole (i+1) parseLoop ((posi, mkFunTy g printerArgTy printerResidueTy) :: acc) (i, fragLine, fragCol+1) fragments + | '%' -> + // This allows for things like `printf "%-4.2%"` to compile and print just a `%` + // For now we are adding a warning, but keeping this behavior. + warning(DiagnosticWithText(3376, FSComp.SR.forBadFormatSpecifierGeneral("%"), m)) + collectSpecifierLocation fragLine fragCol 0 + appendToDotnetFormatString "%" + parseLoop acc (i+1, fragLine, fragCol+1) fragments | c -> failwith (FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)) let results = parseLoop [] (0, 0, m.StartColumn) fragments diff --git a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs index 0347e982900..fa320ea3f7a 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/InterpolatedStringsTests.fs @@ -56,4 +56,4 @@ let s = $"...%-%...{0}" """ |> compile |> shouldFail - |> withSingleDiagnostic (Error 3376, Line 2, Col 9, Line 2, Col 24, "Invalid interpolated string. Bad format specifier: '%'") \ No newline at end of file + |> withSingleDiagnostic (Warning 3376, Line 2, Col 9, Line 2, Col 24, "Bad format specifier: '%'") \ No newline at end of file From a3fb994f7b363a5f534073e45008448f166aecf1 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 10 Feb 2023 19:53:00 +0100 Subject: [PATCH 5/6] Fix editor test --- tests/service/EditorTests.fs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/service/EditorTests.fs b/tests/service/EditorTests.fs index 2c9b12da11a..b656b780f3b 100644 --- a/tests/service/EditorTests.fs +++ b/tests/service/EditorTests.fs @@ -474,7 +474,15 @@ let _ = printf " %*a" 3 (fun _ _ -> ()) 2 let file = "/home/user/Test.fsx" let parseResult, typeCheckResults = parseAndCheckScript(file, input) - typeCheckResults.Diagnostics |> shouldEqual [||] + typeCheckResults.Diagnostics + |> Array.map (fun d -> d.ErrorNumber, d.StartLine, d.StartColumn, d.EndLine, d.EndColumn, d.Message) + |> shouldEqual [| + (3376, 23, 16, 23, 22, "Bad format specifier: '%'") + (3376, 24, 16, 24, 24, "Bad format specifier: '%'") + (3376, 25, 16, 25, 26, "Bad format specifier: '%'") + (3376, 27, 16, 27, 28, "Bad format specifier: '%'") + (3376, 32, 16, 32, 33, "Bad format specifier: '%'") |] + typeCheckResults.GetFormatSpecifierLocationsAndArity() |> Array.map (fun (range,numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs) |> shouldEqual From c330a21dbe1aea0bafc9753e584243b0cea57a57 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 15 Feb 2023 14:23:25 +0100 Subject: [PATCH 6/6] Fix hardcoded path in unit test --- tests/service/EditorTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/service/EditorTests.fs b/tests/service/EditorTests.fs index b656b780f3b..9a50646ed35 100644 --- a/tests/service/EditorTests.fs +++ b/tests/service/EditorTests.fs @@ -471,7 +471,7 @@ let _ = printf " %a" (fun _ _ -> ()) 2 let _ = printf " %*a" 3 (fun _ _ -> ()) 2 """ - let file = "/home/user/Test.fsx" + let file = System.IO.Path.Combine [| "home"; "user"; "Test.fsx" |] let parseResult, typeCheckResults = parseAndCheckScript(file, input) typeCheckResults.Diagnostics