Skip to content

Provide API that includes printf specifier arities along with ranges #527

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 8, 2016
Merged

Provide API that includes printf specifier arities along with ranges #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b968b5d
Report the number of arguments associated with a printf format specif…
latkin Feb 7, 2016
315a04e
Add new tests for * and %% specifiers and update existing tests
latkin Feb 8, 2016
a35257a
Create dedicated API for new behavior, to maintain API compat
latkin Feb 8, 2016
079bd5c
Don't forget that %a also takes 2 arguments
latkin Feb 8, 2016
01da557
Fix goofs for %a tests
latkin Feb 8, 2016
8964d60
Deprecate original API
latkin Feb 8, 2016
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
37 changes: 20 additions & 17 deletions src/fsharp/CheckFormatStrings.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,27 +193,30 @@ let parseFormatStringInternal (m:range) g (source: string option) fmt bty cty =
checkNoZeroFlag c
checkNoNumericPrefix c

let collectSpecifierLocation relLine relCol =
let collectSpecifierLocation relLine relCol numStdArgs =
let numArgsForSpecifier =
numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0)
match relLine with
| 0 ->
specifierLocations.Add(
Range.mkFileIndexRange m.FileIndex
(Range.mkFileIndexRange m.FileIndex
(Range.mkPos m.StartLine (startCol + offset))
(Range.mkPos m.StartLine (relCol + offset)))
(Range.mkPos m.StartLine (relCol + offset))), numArgsForSpecifier)
| _ ->
specifierLocations.Add(
Range.mkFileIndexRange m.FileIndex
(Range.mkFileIndexRange m.FileIndex
(Range.mkPos (m.StartLine + relLine) startCol)
(Range.mkPos (m.StartLine + relLine) relCol))
(Range.mkPos (m.StartLine + relLine) relCol)), numArgsForSpecifier)

let ch = fmt.[i]
match ch with
| '%' ->
| '%' ->
collectSpecifierLocation relLine relCol 0
parseLoop acc (i+1, relLine, relCol+1)

| ('d' | 'i' | 'o' | 'u' | 'x' | 'X') ->
if info.precision then failwithf "%s" <| FSComp.SR.forFormatDoesntSupportPrecision(ch.ToString())
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i+1, relLine, relCol+1)

| ('l' | 'L') ->
Expand All @@ -228,59 +231,59 @@ let parseFormatStringInternal (m:range) g (source: string option) fmt bty cty =
failwithf "%s" <| FSComp.SR.forLIsUnnecessary()
match fmt.[i] with
| ('d' | 'i' | 'o' | 'u' | 'x' | 'X') ->
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i+1, relLine, relCol+1)
| _ -> failwithf "%s" <| FSComp.SR.forBadFormatSpecifier()

| ('h' | 'H') ->
failwithf "%s" <| FSComp.SR.forHIsUnnecessary()

| 'M' ->
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, g.decimal_ty) :: acc) (i+1, relLine, relCol+1)

| ('f' | 'F' | 'e' | 'E' | 'g' | 'G') ->
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, mkFlexibleFloatFormatTypar g m) :: acc) (i+1, relLine, relCol+1)

| 'b' ->
checkOtherFlags ch
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, g.bool_ty) :: acc) (i+1, relLine, relCol+1)

| 'c' ->
checkOtherFlags ch
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, g.char_ty) :: acc) (i+1, relLine, relCol+1)

| 's' ->
checkOtherFlags ch
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, g.string_ty) :: acc) (i+1, relLine, relCol+1)

| 'O' ->
checkOtherFlags ch
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, NewInferenceType ()) :: acc) (i+1, relLine, relCol+1)

| 'A' ->
match info.numPrefixIfPos with
| None // %A has BindingFlags=Public, %+A has BindingFlags=Public | NonPublic
| Some '+' ->
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, NewInferenceType ()) :: acc) (i+1, relLine, relCol+1)
| Some _ -> failwithf "%s" <| FSComp.SR.forDoesNotSupportPrefixFlag(ch.ToString(), (Option.get info.numPrefixIfPos).ToString())

| 'a' ->
checkOtherFlags ch
let xty = NewInferenceType ()
let fty = bty --> (xty --> cty)
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 2
parseLoop ((Option.map ((+)1) posi, xty) :: (posi, fty) :: acc) (i+1, relLine, relCol+1)

| 't' ->
checkOtherFlags ch
collectSpecifierLocation relLine relCol
collectSpecifierLocation relLine relCol 1
parseLoop ((posi, bty --> cty) :: acc) (i+1, relLine, relCol+1)

| c -> failwithf "%s" <| FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/CheckFormatStrings.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ open Microsoft.FSharp.Compiler.Tast
open Microsoft.FSharp.Compiler.TcGlobals
open Microsoft.FSharp.Compiler.AbstractIL.Internal

val ParseFormatString : Range.range -> TcGlobals -> source: string option -> fmt: string -> bty: TType -> cty: TType -> dty: TType -> (TType * TType) * Range.range list
val ParseFormatString : Range.range -> TcGlobals -> source: string option -> fmt: string -> bty: TType -> cty: TType -> dty: TType -> (TType * TType) * (Range.range * int) list

val TryCountFormatStringArguments : m:Range.range -> g:TcGlobals -> fmt:string -> bty:TType -> cty:TType -> int option
10 changes: 5 additions & 5 deletions src/fsharp/NameResolution.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ type ITypecheckResultsSink =
abstract NotifyEnvWithScope : range * NameResolutionEnv * AccessorDomain -> unit
abstract NotifyExprHasType : pos * TType * Tastops.DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
abstract NotifyNameResolution : pos * Item * Item * ItemOccurence * Tastops.DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
abstract NotifyFormatSpecifierLocation : range -> unit
abstract NotifyFormatSpecifierLocation : range * int -> unit
abstract CurrentSource : string option

let (|ValRefOfProp|_|) (pi : PropInfo) = pi.ArbitraryValRef
Expand Down Expand Up @@ -1301,7 +1301,7 @@ type TcResolutions


/// Represents container for all name resolutions that were met so far when typechecking some particular file
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: range[]) =
type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolution>, formatSpecifierLocations: (range * int)[]) =

member this.GetUsesOfSymbol(item) =
[| for cnr in capturedNameResolutions do
Expand All @@ -1312,7 +1312,7 @@ type TcSymbolUses(g, capturedNameResolutions : ResizeArray<CapturedNameResolutio
[| for cnr in capturedNameResolutions do
yield (cnr.Item, cnr.ItemOccurence, cnr.DisplayEnv, cnr.Range) |]

member this.GetFormatSpecifierLocations() = formatSpecifierLocations
member this.GetFormatSpecifierLocationsAndArity() = formatSpecifierLocations


/// An accumulator for the results being emitted into the tcSink.
Expand Down Expand Up @@ -1367,8 +1367,8 @@ type TcResultsSinkImpl(g, ?source: string) =
capturedNameResolutions.Add(CapturedNameResolution(endPos,item,occurenceType,denv,nenv,ad,m))
capturedMethodGroupResolutions.Add(CapturedNameResolution(endPos,itemMethodGroup,occurenceType,denv,nenv,ad,m))

member sink.NotifyFormatSpecifierLocation(m) =
capturedFormatSpecifierLocations.Add(m)
member sink.NotifyFormatSpecifierLocation(m, numArgs) =
capturedFormatSpecifierLocations.Add((m, numArgs))

member sink.CurrentSource = source

Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/NameResolution.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@ type internal TcSymbolUses =

member GetAllUsesOfSymbols : unit -> (Item * ItemOccurence * DisplayEnv * range)[]

member GetFormatSpecifierLocations : unit -> range[]
member GetFormatSpecifierLocationsAndArity : unit -> (range * int)[]


/// An abstract type for reporting the results of name resolution and type checking
type ITypecheckResultsSink =
abstract NotifyEnvWithScope : range * NameResolutionEnv * AccessorDomain -> unit
abstract NotifyExprHasType : pos * TType * DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
abstract NotifyNameResolution : pos * Item * Item * ItemOccurence * DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
abstract NotifyFormatSpecifierLocation : range -> unit
abstract NotifyFormatSpecifierLocation : range * int -> unit
abstract CurrentSource : string option

type internal TcResultsSinkImpl =
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/TypeChecker.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ let MakeAndPublishSimpleVals cenv env m names mergeNamesInOneNameresEnv =
if not m.IsSynthetic then
nameResolutions.Add(pos, a, b, occurence, denv, nenv, ad, m)
member this.NotifyExprHasType(_, _, _, _, _, _) = assert false // no expr typings in MakeSimpleVals
member this.NotifyFormatSpecifierLocation _ = ()
member this.NotifyFormatSpecifierLocation(_, _) = ()
member this.CurrentSource = None }

use _h = WithNewTypecheckResultsSink(sink, cenv.tcSink)
Expand Down Expand Up @@ -6314,8 +6314,8 @@ and TcConstStringExpr cenv overallTy env m tpenv s =
match cenv.tcSink.CurrentSink with
| None -> ()
| Some sink ->
for specifierLocation in specifierLocations do
sink.NotifyFormatSpecifierLocation specifierLocation
for specifierLocation,numArgs in specifierLocations do
sink.NotifyFormatSpecifierLocation(specifierLocation, numArgs)

UnifyTypes cenv env m aty aty'
UnifyTypes cenv env m ety ety'
Expand Down
16 changes: 9 additions & 7 deletions src/fsharp/vs/service.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1472,11 +1472,11 @@ type TypeCheckInfo
[ for x in tcImports.GetImportedAssemblies() do
yield FSharpAssembly(g, tcImports, x.FSharpViewOfMetadata) ]

// Not, this does not have to be a SyncOp, it can be called from any thread
member scope.GetFormatSpecifierLocations() =
sSymbolUses.GetFormatSpecifierLocations()
// Note, this does not have to be a SyncOp, it can be called from any thread
member scope.GetFormatSpecifierLocationsAndArity() =
sSymbolUses.GetFormatSpecifierLocationsAndArity()

// Not, this does not have to be a SyncOp, it can be called from any thread
// Note, this does not have to be a SyncOp, it can be called from any thread
member scope.GetExtraColorizations() =
[| for cnr in sResolutions.CapturedNameResolutions do
match cnr with
Expand Down Expand Up @@ -2053,13 +2053,15 @@ type FSharpCheckFileResults(errors: FSharpErrorInfo[], scopeOptX: TypeCheckInfo
scope.GetSymbolUseAtLocation (line, lineStr, colAtEndOfNames, names)
|> Option.map (fun (sym,_,_) -> sym))


member info.GetFormatSpecifierLocations() =
info.GetFormatSpecifierLocationsAndArity() |> Array.map fst

member info.GetFormatSpecifierLocationsAndArity() =
threadSafeOp
(fun () -> [| |])
(fun (scope, _builder, _reactor) ->
// This operation is not asynchronous - GetFormatSpecifierLocations can be run on the calling thread
scope.GetFormatSpecifierLocations())
// This operation is not asynchronous - GetFormatSpecifierLocationsAndArity can be run on the calling thread
scope.GetFormatSpecifierLocationsAndArity())

member info.GetExtraColorizationsAlternate() =
threadSafeOp
Expand Down
4 changes: 4 additions & 0 deletions src/fsharp/vs/service.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,12 @@ type FSharpCheckFileResults =
member GetExtraColorizationsAlternate : unit -> (range * FSharpTokenColorKind)[]

/// <summary>Get the locations of format specifiers</summary>
[<System.Obsolete("This member has been replaced by GetFormatSpecifierLocationsAndArity, which returns both range and arity of specifiers")>]
member GetFormatSpecifierLocations : unit -> range[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Obsolete to this, with a message to use the new method


/// <summary>Get the locations of and number of arguments associated with format specifiers</summary>
member GetFormatSpecifierLocationsAndArity : unit -> (range*int)[]

/// Get all textual usages of all symbols throughout the file
member GetAllUsesOfAllSymbolsInFile : unit -> Async<FSharpSymbolUse[]>

Expand Down
97 changes: 64 additions & 33 deletions tests/service/EditorTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,30 +385,58 @@ let _ = List.map (sprintf @"%A
let _ = (10, 12) ||> sprintf "%A
%O"
let _ = sprintf "\n%-8.1e+567" 1.0
let _ = sprintf @"%O\n%-5s" "1" "2" """
let _ = sprintf @"%O\n%-5s" "1" "2"
let _ = sprintf "%%"
let _ = sprintf " %*%" 2
let _ = sprintf " %.*%" 2
let _ = sprintf " %*.1%" 2
let _ = sprintf " %*s" 10 "hello"
let _ = sprintf " %*.*%" 2 3
let _ = sprintf " %*.*f" 2 3 4.5
let _ = sprintf " %.*f" 3 4.5
let _ = sprintf " %*.1f" 3 4.5
let _ = sprintf " %6.*f" 3 4.5
let _ = sprintf " %6.*%" 3
let _ = printf " %a" (fun _ _ -> ()) 2
let _ = printf " %*a" 3 (fun _ _ -> ()) 2
"""

let file = "/home/user/Test.fsx"
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)

typeCheckResults.Errors |> shouldEqual [||]
typeCheckResults.GetFormatSpecifierLocations()
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
|> shouldEqual [|(2, 45, 2, 46);
(3, 23, 3, 24);
(4, 38, 4, 39);
(5, 29, 5, 30);
(6, 17, 6, 19);
(7, 17, 7, 21);
(8, 17, 8, 22);
(9, 18, 9, 21);
(10, 18, 10, 20);
(12, 12, 12, 14);
(15, 12, 15, 14);
(16, 28, 16, 29);
(18, 30, 18, 31);
(19, 30, 19, 31);
(20, 19, 20, 24);
(21, 18, 21, 19); (21, 22, 21, 25)|]
typeCheckResults.GetFormatSpecifierLocationsAndArity()
|> Array.map (fun (range,numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
|> shouldEqual [|(2, 45, 2, 46, 1);
(3, 23, 3, 24, 1);
(4, 38, 4, 39, 1);
(5, 29, 5, 30, 1);
(6, 17, 6, 19, 2);
(7, 17, 7, 21, 1);
(8, 17, 8, 22, 1);
(9, 18, 9, 21, 1);
(10, 18, 10, 20, 1);
(12, 12, 12, 14, 1);
(15, 12, 15, 14, 1);
(16, 28, 16, 29, 1);
(18, 30, 18, 31, 1);
(19, 30, 19, 31, 1);
(20, 19, 20, 24, 1);
(21, 18, 21, 19, 1);
(21, 22, 21, 25, 1);
(22, 17, 22, 18, 0);
(23, 18, 23, 20, 1);
(24, 19, 24, 22, 1);
(25, 20, 25, 24, 1);
(26, 21, 26, 23, 2);
(27, 22, 27, 26, 2);
(28, 23, 28, 27, 3);
(29, 24, 29, 27, 2);
(30, 25, 30, 29, 2);
(31, 26, 31, 30, 2);
(32, 27, 32, 31, 1);
(33, 28, 33, 29, 2);
(34, 29, 34, 31, 3)|]

[<Test>]
let ``Printf specifiers for triple-quote strings`` () =
Expand All @@ -426,12 +454,13 @@ let _ = List.iter(printfn \"\"\"%-A
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)

typeCheckResults.Errors |> shouldEqual [||]
typeCheckResults.GetFormatSpecifierLocations()
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
|> shouldEqual [|(2, 19, 2, 21);
(4, 12, 4, 14);
(6, 29, 6, 31);
(7, 29, 7, 30); (7, 33, 7, 34)|]
typeCheckResults.GetFormatSpecifierLocationsAndArity()
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
|> shouldEqual [|(2, 19, 2, 21, 1);
(4, 12, 4, 14, 1);
(6, 29, 6, 31, 1);
(7, 29, 7, 30, 1);
(7, 33, 7, 34, 1)|]

[<Test>]
let ``Printf specifiers for user-defined functions`` () =
Expand All @@ -446,25 +475,27 @@ let _ = debug "[LanguageService] Type checking fails for '%s' with content=%A an
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)

typeCheckResults.Errors |> shouldEqual [||]
typeCheckResults.GetFormatSpecifierLocations()
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
|> shouldEqual [|(3, 24, 3, 25);
(3, 29, 3, 30);
(4, 58, 4, 59); (4, 75, 4, 76); (4, 82, 4, 83); (4, 108, 4, 109)|]
typeCheckResults.GetFormatSpecifierLocationsAndArity()
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
|> shouldEqual [|(3, 24, 3, 25, 1);
(3, 29, 3, 30, 1);
(4, 58, 4, 59, 1);
(4, 75, 4, 76, 1);
(4, 82, 4, 83, 1);
(4, 108, 4, 109, 1)|]

[<Test>]
let ``should not report format specifiers for illformed format strings`` () =
let input =
"""
let _ = sprintf "%.7f %7.1A %7.f %--8.1f"
let _ = sprintf "%%A"
let _ = sprintf "ABCDE"
"""

let file = "/home/user/Test.fsx"
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)
typeCheckResults.GetFormatSpecifierLocations()
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
typeCheckResults.GetFormatSpecifierLocationsAndArity()
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
|> shouldEqual [||]

[<Test>]
Expand Down