Skip to content

Improve reporting of format specifiers #279

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
fsgit merged 4 commits into from
Nov 30, 2014
Merged

Improve reporting of format specifiers #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be41d77
Handle '\n' in format strings correctly
dungpa Nov 27, 2014
5a2d47a
Only report specifier locations if format strings are wellformed
dungpa Nov 28, 2014
7e3878e
Adjust unit tests
dungpa Nov 28, 2014
dd2793c
Minor updates
dungpa Nov 30, 2014
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
70 changes: 40 additions & 30 deletions src/fsharp/formats.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ let newInfo ()=
precision = false}

let ParseFormatString (m: Range.range) g (source: string option) report fmt bty cty dty =
let len = String.length fmt

// Offset to adjust ranges depending on whether input string is regular, verbatim or triple-quote
let offset =
// Offset is used to adjust ranges depending on whether input string is regular, verbatim or triple-quote.
// We construct a new 'fmt' string since the current 'fmt' string doesn't distinguish between "\n" and escaped "\\n".
let (offset, fmt) =
match source with
| Some source ->
let source = source.Replace("\r\n", "\n").Replace("\r", "\n")
Expand All @@ -62,15 +61,20 @@ let ParseFormatString (m: Range.range) g (source: string option) report fmt bty
|> Seq.scan (+) 0
|> Seq.toArray
let length = source.Length
if m.StartLine < positions.Length then
if m.EndLine < positions.Length then
let startIndex = positions.[m.StartLine-1] + m.StartColumn
if startIndex <= length-3 && source.[startIndex..startIndex+2] = "\"\"\"" then
3
elif startIndex <= length-2 && source.[startIndex..startIndex+1] = "@\"" then
2
else 1
else 1
| None -> 1
let endIndex = positions.[m.EndLine-1] + m.EndColumn - 1
if startIndex < length-3 && source.[startIndex..startIndex+2] = "\"\"\"" then
(3, source.[startIndex+3..endIndex-3])
elif startIndex < length-2 && source.[startIndex..startIndex+1] = "@\"" then
(2, source.[startIndex+2..endIndex-1])
else (1, source.[startIndex+1..endIndex-1])
else (1, fmt)
| None -> (1, fmt)

let len = String.length fmt

let specifierLocations = ResizeArray()

let rec parseLoop acc (i, relLine, relCol) =
if i >= len then
Expand Down Expand Up @@ -191,14 +195,16 @@ let ParseFormatString (m: Range.range) g (source: string option) report fmt bty
checkNoZeroFlag c;
checkNoNumericPrefix c

let reportLocation relLine relCol =
let collectSpecifierLocation relLine relCol =
match relLine with
| 0 ->
report (Range.mkFileIndexRange m.FileIndex
specifierLocations.Add(
Range.mkFileIndexRange m.FileIndex
(Range.mkPos m.StartLine (startCol + offset))
(Range.mkPos m.StartLine (relCol + offset)))
| _ ->
report (Range.mkFileIndexRange m.FileIndex
specifierLocations.Add(
Range.mkFileIndexRange m.FileIndex
(Range.mkPos (m.StartLine + relLine) startCol)
(Range.mkPos (m.StartLine + relLine) relCol))

Expand All @@ -209,7 +215,7 @@ let ParseFormatString (m: Range.range) g (source: string option) report fmt bty

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

| ('l' | 'L') ->
Expand All @@ -224,64 +230,68 @@ let ParseFormatString (m: Range.range) g (source: string option) report fmt bty
failwithf "%s" <| FSComp.SR.forLIsUnnecessary()
match fmt.[i] with
| ('d' | 'i' | 'o' | 'u' | 'x' | 'X') ->
reportLocation relLine relCol
collectSpecifierLocation relLine relCol
parseLoop ((posi, mkFlexibleIntFormatTypar g m) :: acc) (i+1, relLine, relCol+1)
| _ -> failwithf "%s" <| FSComp.SR.forBadFormatSpecifier()

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

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

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

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

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

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

| 'O' ->
checkOtherFlags ch;
reportLocation relLine relCol
collectSpecifierLocation relLine relCol
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 '+' ->
reportLocation relLine relCol
collectSpecifierLocation relLine relCol
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)
reportLocation relLine relCol
collectSpecifierLocation relLine relCol
parseLoop ((Option.map ((+)1) posi, xty) :: (posi, fty) :: acc) (i+1, relLine, relCol+1)

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

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

| '\n' -> parseLoop acc (i+1, relLine+1, 0)
| _ -> parseLoop acc (i+1, relLine, relCol+1)
parseLoop [] (0, 0, m.StartColumn)

| '\n' -> parseLoop acc (i+1, relLine+1, 0)
| _ -> parseLoop acc (i+1, relLine, relCol+1)

let results = parseLoop [] (0, 0, m.StartColumn)
// Only report specifier locations if entire format strings are well-formed.
for specifierLocation in specifierLocations do
report specifierLocation
results
40 changes: 30 additions & 10 deletions tests/service/EditorTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,11 @@ 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 file = "/home/user/Test.fsx"
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)

typeCheckResults.Errors |> shouldEqual [||]
typeCheckResults.GetFormatSpecifierLocations()
Expand All @@ -367,7 +368,9 @@ let _ = (10, 12) ||> sprintf "%A
(15, 12, 15, 14);
(16, 28, 16, 29);
(18, 30, 18, 31);
(19, 30, 19, 31)|]
(19, 30, 19, 31);
(20, 19, 20, 24);
(21, 18, 21, 19); (21, 22, 21, 25)|]

[<Test>]
let ``Printf specifiers for triple-quote strings`` () =
Expand All @@ -378,34 +381,51 @@ let _ = printfn \"\"\"
%-A
\"\"\" -10
let _ = List.iter(printfn \"\"\"%-A

\"\"\")
"
%i\\n%O
\"\"\" 1 2)"

let file = "/home/user/Test.fsx"
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)
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)|]
(6, 29, 6, 31);
(7, 29, 7, 30); (7, 33, 7, 34)|]

[<Test>]
let ``Printf specifiers for user-defined functions`` () =
let input =
"""
let debug msg = Printf.kprintf System.Diagnostics.Debug.WriteLine msg
let _ = debug "Message: %i - %O" 1 "Ok"
let _ = debug "[LanguageService] Type checking fails for '%s' with content=%A and %A.\nResulting exception: %A" "1" "2" "3" "4"
"""

let file = "/home/user/Test.fsx"
let untyped, typeCheckResults = parseAndTypeCheckFileInProject(file, input)
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)|]
(3, 29, 3, 30);
(4, 58, 4, 59); (4, 75, 4, 76); (4, 82, 4, 83); (4, 108, 4, 109)|]

[<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)
|> shouldEqual [||]