Skip to content

Commit cbfe460

Browse files
dsymedsyme
committed
Merge pull request #527 from latkin/latkin-report-specifier-arity
Provide API that includes printf specifier arities along with ranges
2 parents 1bbcf25 + 8964d60 commit cbfe460

File tree

8 files changed

+108
-68
lines changed

8 files changed

+108
-68
lines changed

‎src/fsharp/CheckFormatStrings.fs‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -193,27 +193,30 @@ let parseFormatStringInternal (m:range) g (source: string option) fmt bty cty =
193193
checkNoZeroFlag c
194194
checkNoNumericPrefix c
195195

196-
let collectSpecifierLocation relLine relCol =
196+
let collectSpecifierLocation relLine relCol numStdArgs =
197+
let numArgsForSpecifier =
198+
numStdArgs + (if widthArg then 1 else 0) + (if precisionArg then 1 else 0)
197199
match relLine with
198200
| 0 ->
199201
specifierLocations.Add(
200-
Range.mkFileIndexRange m.FileIndex
202+
(Range.mkFileIndexRange m.FileIndex
201203
(Range.mkPos m.StartLine (startCol + offset))
202-
(Range.mkPos m.StartLine (relCol + offset)))
204+
(Range.mkPos m.StartLine (relCol + offset))), numArgsForSpecifier)
203205
| _ ->
204206
specifierLocations.Add(
205-
Range.mkFileIndexRange m.FileIndex
207+
(Range.mkFileIndexRange m.FileIndex
206208
(Range.mkPos (m.StartLine + relLine) startCol)
207-
(Range.mkPos (m.StartLine + relLine) relCol))
209+
(Range.mkPos (m.StartLine + relLine) relCol)), numArgsForSpecifier)
208210

209211
let ch = fmt.[i]
210212
match ch with
211-
| '%' ->
213+
| '%' ->
214+
collectSpecifierLocation relLine relCol 0
212215
parseLoop acc (i+1, relLine, relCol+1)
213216

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

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

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

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

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

246249
| 'b' ->
247250
checkOtherFlags ch
248-
collectSpecifierLocation relLine relCol
251+
collectSpecifierLocation relLine relCol 1
249252
parseLoop ((posi, g.bool_ty) :: acc) (i+1, relLine, relCol+1)
250253

251254
| 'c' ->
252255
checkOtherFlags ch
253-
collectSpecifierLocation relLine relCol
256+
collectSpecifierLocation relLine relCol 1
254257
parseLoop ((posi, g.char_ty) :: acc) (i+1, relLine, relCol+1)
255258

256259
| 's' ->
257260
checkOtherFlags ch
258-
collectSpecifierLocation relLine relCol
261+
collectSpecifierLocation relLine relCol 1
259262
parseLoop ((posi, g.string_ty) :: acc) (i+1, relLine, relCol+1)
260263

261264
| 'O' ->
262265
checkOtherFlags ch
263-
collectSpecifierLocation relLine relCol
266+
collectSpecifierLocation relLine relCol 1
264267
parseLoop ((posi, NewInferenceType ()) :: acc) (i+1, relLine, relCol+1)
265268

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

274277
| 'a' ->
275278
checkOtherFlags ch
276279
let xty = NewInferenceType ()
277280
let fty = bty --> (xty --> cty)
278-
collectSpecifierLocation relLine relCol
281+
collectSpecifierLocation relLine relCol 2
279282
parseLoop ((Option.map ((+)1) posi, xty) :: (posi, fty) :: acc) (i+1, relLine, relCol+1)
280283

281284
| 't' ->
282285
checkOtherFlags ch
283-
collectSpecifierLocation relLine relCol
286+
collectSpecifierLocation relLine relCol 1
284287
parseLoop ((posi, bty --> cty) :: acc) (i+1, relLine, relCol+1)
285288

286289
| c -> failwithf "%s" <| FSComp.SR.forBadFormatSpecifierGeneral(String.make 1 c)

‎src/fsharp/CheckFormatStrings.fsi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ open Microsoft.FSharp.Compiler.Tast
1313
open Microsoft.FSharp.Compiler.TcGlobals
1414
open Microsoft.FSharp.Compiler.AbstractIL.Internal
1515

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

1818
val TryCountFormatStringArguments : m:Range.range -> g:TcGlobals -> fmt:string -> bty:TType -> cty:TType -> int option

‎src/fsharp/NameResolution.fs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,7 @@ type ITypecheckResultsSink =
11051105
abstract NotifyEnvWithScope : range * NameResolutionEnv * AccessorDomain -> unit
11061106
abstract NotifyExprHasType : pos * TType * Tastops.DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
11071107
abstract NotifyNameResolution : pos * Item * Item * ItemOccurence * Tastops.DisplayEnv * NameResolutionEnv * AccessorDomain * range -> unit
1108-
abstract NotifyFormatSpecifierLocation : range -> unit
1108+
abstract NotifyFormatSpecifierLocation : range * int -> unit
11091109
abstract CurrentSource : string option
11101110

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

13021302

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

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

1315-
member this.GetFormatSpecifierLocations() = formatSpecifierLocations
1315+
member this.GetFormatSpecifierLocationsAndArity() = formatSpecifierLocations
13161316

13171317

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

1370-
member sink.NotifyFormatSpecifierLocation(m) =
1371-
capturedFormatSpecifierLocations.Add(m)
1370+
member sink.NotifyFormatSpecifierLocation(m, numArgs) =
1371+
capturedFormatSpecifierLocations.Add((m, numArgs))
13721372

13731373
member sink.CurrentSource = source
13741374

‎src/fsharp/NameResolution.fsi‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,15 @@ type internal TcSymbolUses =
233233

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

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

238238

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

247247
type internal TcResultsSinkImpl =

‎src/fsharp/TypeChecker.fs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,7 +1739,7 @@ let MakeAndPublishSimpleVals cenv env m names mergeNamesInOneNameresEnv =
17391739
if not m.IsSynthetic then
17401740
nameResolutions.Add(pos, a, b, occurence, denv, nenv, ad, m)
17411741
member this.NotifyExprHasType(_, _, _, _, _, _) = assert false // no expr typings in MakeSimpleVals
1742-
member this.NotifyFormatSpecifierLocation _ = ()
1742+
member this.NotifyFormatSpecifierLocation(_, _) = ()
17431743
member this.CurrentSource = None }
17441744

17451745
use _h = WithNewTypecheckResultsSink(sink, cenv.tcSink)
@@ -6314,8 +6314,8 @@ and TcConstStringExpr cenv overallTy env m tpenv s =
63146314
match cenv.tcSink.CurrentSink with
63156315
| None -> ()
63166316
| Some sink ->
6317-
for specifierLocation in specifierLocations do
6318-
sink.NotifyFormatSpecifierLocation specifierLocation
6317+
for specifierLocation,numArgs in specifierLocations do
6318+
sink.NotifyFormatSpecifierLocation(specifierLocation, numArgs)
63196319

63206320
UnifyTypes cenv env m aty aty'
63216321
UnifyTypes cenv env m ety ety'

‎src/fsharp/vs/service.fs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,11 +1472,11 @@ type TypeCheckInfo
14721472
[ for x in tcImports.GetImportedAssemblies() do
14731473
yield FSharpAssembly(g, tcImports, x.FSharpViewOfMetadata) ]
14741474

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

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

2056-
20572056
member info.GetFormatSpecifierLocations() =
2057+
info.GetFormatSpecifierLocationsAndArity() |> Array.map fst
2058+
2059+
member info.GetFormatSpecifierLocationsAndArity() =
20582060
threadSafeOp
20592061
(fun () -> [| |])
20602062
(fun (scope, _builder, _reactor) ->
2061-
// This operation is not asynchronous - GetFormatSpecifierLocations can be run on the calling thread
2062-
scope.GetFormatSpecifierLocations())
2063+
// This operation is not asynchronous - GetFormatSpecifierLocationsAndArity can be run on the calling thread
2064+
scope.GetFormatSpecifierLocationsAndArity())
20632065

20642066
member info.GetExtraColorizationsAlternate() =
20652067
threadSafeOp

‎src/fsharp/vs/service.fsi‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,12 @@ type FSharpCheckFileResults =
260260
member GetExtraColorizationsAlternate : unit -> (range * FSharpTokenColorKind)[]
261261

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

266+
/// <summary>Get the locations of and number of arguments associated with format specifiers</summary>
267+
member GetFormatSpecifierLocationsAndArity : unit -> (range*int)[]
268+
265269
/// Get all textual usages of all symbols throughout the file
266270
member GetAllUsesOfAllSymbolsInFile : unit -> Async<FSharpSymbolUse[]>
267271

‎tests/service/EditorTests.fs‎

Lines changed: 64 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -385,30 +385,58 @@ let _ = List.map (sprintf @"%A
385385
let _ = (10, 12) ||> sprintf "%A
386386
%O"
387387
let _ = sprintf "\n%-8.1e+567" 1.0
388-
let _ = sprintf @"%O\n%-5s" "1" "2" """
388+
let _ = sprintf @"%O\n%-5s" "1" "2"
389+
let _ = sprintf "%%"
390+
let _ = sprintf " %*%" 2
391+
let _ = sprintf " %.*%" 2
392+
let _ = sprintf " %*.1%" 2
393+
let _ = sprintf " %*s" 10 "hello"
394+
let _ = sprintf " %*.*%" 2 3
395+
let _ = sprintf " %*.*f" 2 3 4.5
396+
let _ = sprintf " %.*f" 3 4.5
397+
let _ = sprintf " %*.1f" 3 4.5
398+
let _ = sprintf " %6.*f" 3 4.5
399+
let _ = sprintf " %6.*%" 3
400+
let _ = printf " %a" (fun _ _ -> ()) 2
401+
let _ = printf " %*a" 3 (fun _ _ -> ()) 2
402+
"""
389403

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

393407
typeCheckResults.Errors |> shouldEqual [||]
394-
typeCheckResults.GetFormatSpecifierLocations()
395-
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
396-
|> shouldEqual [|(2, 45, 2, 46);
397-
(3, 23, 3, 24);
398-
(4, 38, 4, 39);
399-
(5, 29, 5, 30);
400-
(6, 17, 6, 19);
401-
(7, 17, 7, 21);
402-
(8, 17, 8, 22);
403-
(9, 18, 9, 21);
404-
(10, 18, 10, 20);
405-
(12, 12, 12, 14);
406-
(15, 12, 15, 14);
407-
(16, 28, 16, 29);
408-
(18, 30, 18, 31);
409-
(19, 30, 19, 31);
410-
(20, 19, 20, 24);
411-
(21, 18, 21, 19); (21, 22, 21, 25)|]
408+
typeCheckResults.GetFormatSpecifierLocationsAndArity()
409+
|> Array.map (fun (range,numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
410+
|> shouldEqual [|(2, 45, 2, 46, 1);
411+
(3, 23, 3, 24, 1);
412+
(4, 38, 4, 39, 1);
413+
(5, 29, 5, 30, 1);
414+
(6, 17, 6, 19, 2);
415+
(7, 17, 7, 21, 1);
416+
(8, 17, 8, 22, 1);
417+
(9, 18, 9, 21, 1);
418+
(10, 18, 10, 20, 1);
419+
(12, 12, 12, 14, 1);
420+
(15, 12, 15, 14, 1);
421+
(16, 28, 16, 29, 1);
422+
(18, 30, 18, 31, 1);
423+
(19, 30, 19, 31, 1);
424+
(20, 19, 20, 24, 1);
425+
(21, 18, 21, 19, 1);
426+
(21, 22, 21, 25, 1);
427+
(22, 17, 22, 18, 0);
428+
(23, 18, 23, 20, 1);
429+
(24, 19, 24, 22, 1);
430+
(25, 20, 25, 24, 1);
431+
(26, 21, 26, 23, 2);
432+
(27, 22, 27, 26, 2);
433+
(28, 23, 28, 27, 3);
434+
(29, 24, 29, 27, 2);
435+
(30, 25, 30, 29, 2);
436+
(31, 26, 31, 30, 2);
437+
(32, 27, 32, 31, 1);
438+
(33, 28, 33, 29, 2);
439+
(34, 29, 34, 31, 3)|]
412440

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

428456
typeCheckResults.Errors |> shouldEqual [||]
429-
typeCheckResults.GetFormatSpecifierLocations()
430-
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
431-
|> shouldEqual [|(2, 19, 2, 21);
432-
(4, 12, 4, 14);
433-
(6, 29, 6, 31);
434-
(7, 29, 7, 30); (7, 33, 7, 34)|]
457+
typeCheckResults.GetFormatSpecifierLocationsAndArity()
458+
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
459+
|> shouldEqual [|(2, 19, 2, 21, 1);
460+
(4, 12, 4, 14, 1);
461+
(6, 29, 6, 31, 1);
462+
(7, 29, 7, 30, 1);
463+
(7, 33, 7, 34, 1)|]
435464

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

448477
typeCheckResults.Errors |> shouldEqual [||]
449-
typeCheckResults.GetFormatSpecifierLocations()
450-
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
451-
|> shouldEqual [|(3, 24, 3, 25);
452-
(3, 29, 3, 30);
453-
(4, 58, 4, 59); (4, 75, 4, 76); (4, 82, 4, 83); (4, 108, 4, 109)|]
478+
typeCheckResults.GetFormatSpecifierLocationsAndArity()
479+
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
480+
|> shouldEqual [|(3, 24, 3, 25, 1);
481+
(3, 29, 3, 30, 1);
482+
(4, 58, 4, 59, 1);
483+
(4, 75, 4, 76, 1);
484+
(4, 82, 4, 83, 1);
485+
(4, 108, 4, 109, 1)|]
454486

455487
[<Test>]
456488
let ``should not report format specifiers for illformed format strings`` () =
457489
let input =
458490
"""
459491
let _ = sprintf "%.7f %7.1A %7.f %--8.1f"
460-
let _ = sprintf "%%A"
461492
let _ = sprintf "ABCDE"
462493
"""
463494

464495
let file = "/home/user/Test.fsx"
465496
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)
466-
typeCheckResults.GetFormatSpecifierLocations()
467-
|> Array.map (fun range -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn)
497+
typeCheckResults.GetFormatSpecifierLocationsAndArity()
498+
|> Array.map (fun (range, numArgs) -> range.StartLine, range.StartColumn, range.EndLine, range.EndColumn, numArgs)
468499
|> shouldEqual [||]
469500

470501
[<Test>]

0 commit comments

Comments
 (0)