Skip to content
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.

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

Add a codefix that suggests replacements for unknown identifiers #2106

Merged
merged 6 commits into from
Dec 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 9 additions & 12 deletions src/fsharp/ErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,17 @@ let minStringLengthForThreshold = 3

let thresholdForSuggestions = 0.7

/// Filters predictions based on edit distance to an unknown identifier.
/// Filters predictions based on edit distance to the given unknown identifier.
let FilterPredictions (unknownIdent:string) (predictionsF:ErrorLogger.Suggestions) =
let unknownIdent = unknownIdent.ToUpperInvariant()
let useThreshold = unknownIdent.Length >= minStringLengthForThreshold
predictionsF()
|> Seq.choose (fun p ->
let similarity = Internal.Utilities.EditDistance.JaroWinklerDistance unknownIdent (p.ToUpperInvariant())
if not useThreshold || similarity >= thresholdForSuggestions then
if not useThreshold || similarity >= thresholdForSuggestions || p.Contains unknownIdent then
Some(similarity,p)
else
None
)
None)
|> Seq.sortByDescending fst
|> Seq.mapi (fun i x -> i,x)
|> Seq.takeWhile (fun (i,_) -> i < maxSuggestions)
Expand All @@ -31,19 +30,17 @@ let FormatPredictions errorStyle normalizeF (predictions: (float * string) list)
match predictions with
| [] -> System.String.Empty
| _ ->
" " + FSComp.SR.undefinedNameSuggestionsIntro() +
match errorStyle with
| ErrorLogger.ErrorStyle.VSErrors ->
let predictionText =
predictions
|> List.map (snd >> normalizeF)
|> String.concat ", "

" " + FSComp.SR.undefinedNameRecordLabelDetails() + " " + predictionText
" " + predictionText
| _ ->
let predictionText =
predictions
|> List.map (snd >> normalizeF)
|> Seq.map (sprintf "%s %s" System.Environment.NewLine)
|> String.concat ""

System.Environment.NewLine + FSComp.SR.undefinedNameRecordLabelDetails() + predictionText
predictions
|> List.map (snd >> normalizeF)
|> List.map (sprintf "%s %s" System.Environment.NewLine)
|> String.concat ""
3 changes: 2 additions & 1 deletion src/fsharp/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ undefinedNameType,"The type '%s' is not defined."
undefinedNameTypeIn,"The type '%s' is not defined in '%s'."
undefinedNameRecordLabelOrNamespace,"The record label or namespace '%s' is not defined."
undefinedNameRecordLabel,"The record label '%s' is not defined."
undefinedNameRecordLabelDetails,"Maybe you want one of the following:"
undefinedNameSuggestionsIntro,"Maybe you want one of the following:"
undefinedNameTypeParameter,"The type parameter '%s is not defined."
undefinedNamePatternDiscriminator,"The pattern discriminator '%s' is not defined."
replaceWithSuggestion,"Replace with '%s'"
missingElseBranch,"The 'if' expression is missing an 'else' branch. The 'then' branch has type '%s'. Because 'if' is an expression, and not a statement, add an 'else' branch which returns a value of the same type."
elseBranchHasWrongType,"All branches of an 'if' expression must return the same type. This expression was expected to have type '%s' but here has type '%s'."
commaInsteadOfSemicolonInRecord,"A ';' is used to separate field values in records. Consider replacing ',' with ';'."
Expand Down
4 changes: 1 addition & 3 deletions tests/fsharp/core/load-script/out.stderr.bsl
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@

usesfsi.fsx(2,1): error FS0039: The namespace or module 'fsi' is not defined.

Maybe you want one of the following:
usesfsi.fsx(2,1): error FS0039: The namespace or module 'fsi' is not defined. Maybe you want one of the following:

FSharp
2 changes: 1 addition & 1 deletion tests/fsharp/typecheck/sigs/neg06.bsl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

neg06.fs(3,40,3,45): typecheck error FS0039: The field, constructor or member 'Ascii' is not defined. Maybe you want one of the following: ASCII
neg06.fs(3,40,3,45): typecheck error FS0039: The field, constructor or member 'Ascii' is not defined. Maybe you want one of the following: ASCII, get_ASCII

neg06.fs(12,6,12,31): typecheck error FS0942: Struct types are always sealed

Expand Down
55 changes: 55 additions & 0 deletions vsintegration/src/FSharp.Editor/CodeFix/ReplaceWithSuggestion.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace rec Microsoft.VisualStudio.FSharp.Editor

open System
open System.Composition
open System.Collections.Immutable
open System.Threading
open System.Threading.Tasks

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes
open Microsoft.CodeAnalysis.CodeActions

[<ExportCodeFixProvider(FSharpCommonConstants.FSharpLanguageName, Name = "ReplaceWithSuggestion"); Shared>]
type internal FSharpReplaceWithSuggestionCodeFixProvider() =
inherit CodeFixProvider()
let fixableDiagnosticIds = ["FS0039"; "FS1129"; "FS0495"] |> Set.ofList
let maybeString = FSComp.SR.undefinedNameSuggestionsIntro()

let createCodeFix (title: string, context: CodeFixContext, textChange: TextChange) =
CodeAction.Create(
title,
(fun (cancellationToken: CancellationToken) ->
async {
let! sourceText = context.Document.GetTextAsync()
return context.Document.WithText(sourceText.WithChanges(textChange))
} |> CommonRoslynHelpers.StartAsyncAsTask(cancellationToken)),
title)

override __.FixableDiagnosticIds = fixableDiagnosticIds.ToImmutableArray()

override __.RegisterCodeFixesAsync context : Task =
async {
context.Diagnostics
|> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
|> Seq.iter (fun diagnostic ->
let message = diagnostic.GetMessage()
let splitted = message.Split([|maybeString|], StringSplitOptions.None)
Copy link
Member

Choose a reason for hiding this comment

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

splitted is not really a word :-)

The past tense of split is .... split.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://en.m.wiktionary.org/wiki/splitted it says I'm nonstandard or archaic. Guess I can live with that.

Anyways, will fix tomorrow.

if splitted.Length > 1 then
Copy link
Member

@KevinRansom KevinRansom Dec 27, 2016

Choose a reason for hiding this comment

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

This feels somewhat icky ... we should consider adding to the compiler service an improved method of transmitting error information to the IDE.

For example:

  • I'm not at all sure how this string parsing we do in these fixers would work on a language like Arabic which works left to right. (hmmm I meant right to left)

Obviously that's not specific to this PR ... but we should consider an improvement.

Kevin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely. Parsing error messages is definitely the wrong way to do it (I think this specific case could also work in Arabic - because we concatenate the error message from left to right).

That said: it is an easy way to do it. And it works in practice. We already love to use it in ionide

let suggestions =
splitted.[1].Split([|' '; '\r'; '\n'|], StringSplitOptions.RemoveEmptyEntries)
|> Array.map (fun s -> s.Trim())

let diagnostics = [| diagnostic |].ToImmutableArray()

for suggestion in suggestions do
let codefix =
createCodeFix(
FSComp.SR.replaceWithSuggestion suggestion,
context,
TextChange(context.Span, suggestion))
context.RegisterCodeFix(codefix, diagnostics))
} |> CommonRoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
<Compile Include="Commands\FsiCommandService.fs" />
<Compile Include="CodeFix\AddNewKeywordToDisposableConstructorInvocation.fs" />
<Compile Include="CodeFix\AddOpenCodeFixProvider.fs" />
<Compile Include="CodeFix\ReplaceWithSuggestion.fs" />
<Compile Include="CodeFix\PrefixUnusedValueWithUnderscore.fs" />
</ItemGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ let x: float = 1.2(*start*).(*end*)3
let gDateTime (arr: (*start*)DateTime(*end*)[]) =
arr.[0]
""",
expectedMessage = "The type 'DateTime' is not defined.\r\nMaybe you want one of the following:\r\n nativeint\r\n DelegateEvent")
expectedMessage = "The type 'DateTime' is not defined. Maybe you want one of the following:\r\n nativeint\r\n DelegateEvent")

[<Test>]
member public this.Error_CyclicalDeclarationDoesNotCrash() =
Expand Down