Skip to content

Commit

Permalink
QuickFix support for spelling errors (Azure#630)
Browse files Browse the repository at this point in the history
* Add quick fix code action handler

* Bump @typescript-eslint/parser to eliminate false positive warnings

* Refactoring and error fixes

* More test coverage for code action handler

* Make spell correction action preferred

* Fix a test failure due to rebasing

* Clean up code

* Add unit tests for diagnostics with code fixes
  • Loading branch information
shenglol authored Oct 13, 2020
1 parent 1119239 commit fe817ab
Show file tree
Hide file tree
Showing 15 changed files with 519 additions and 146 deletions.
38 changes: 38 additions & 0 deletions src/Bicep.Core.UnitTests/Diagnostics/ErrorBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,44 @@ public void DiagnosticBuilder_CodesAreUnique()
}
}

[TestMethod]
public void UnknownPropertyWithSuggestion_ProducesPreferredPropertyReplacementCodeFix()
{
var builder = DiagnosticBuilder.ForPosition(new TextSpan(0, 10));

var diagnostic = builder.UnknownPropertyWithSuggestion(false, new PrimitiveType("testType", TypeSymbolValidationFlags.Default), "networkACLs", "networkAcls");
diagnostic.Fixes.Should().NotBeNull();
diagnostic.Fixes.Should().HaveCount(1);

var fix = diagnostic.Fixes.First();
fix.IsPreferred.Should().BeTrue();
fix.Replacements.Should().NotBeNull();
fix.Replacements.Should().HaveCount(1);

var replacement = fix.Replacements.First();
replacement.Span.Should().Be(diagnostic.Span);
replacement.Text.Should().Be("networkAcls");
}

[TestMethod]
public void SymbolicNameDoesNotExistWithSuggestion_ProducesPreferredNameReplacementCodeFix()
{
var builder = DiagnosticBuilder.ForPosition(new TextSpan(0, 10));

var diagnostic = builder.SymbolicNameDoesNotExistWithSuggestion("hellO", "hello");
diagnostic.Fixes.Should().NotBeNull();
diagnostic.Fixes.Should().HaveCount(1);

var fix = diagnostic.Fixes.First();
fix.IsPreferred.Should().BeTrue();
fix.Replacements.Should().NotBeNull();
fix.Replacements.Should().HaveCount(1);

var replacement = fix.Replacements.First();
replacement.Span.Should().Be(diagnostic.Span);
replacement.Text.Should().Be("hello");
}

private static object CreateMockParameter(ParameterInfo parameter, int index)
{
if (parameter.ParameterType == typeof(TypeSymbol))
Expand Down
50 changes: 50 additions & 0 deletions src/Bicep.Core/CodeAction/CodeFix.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;

namespace Bicep.Core.CodeAction
{
public class CodeFix
{
private readonly CodeReplacement replacement;
private readonly IEnumerable<CodeReplacement>? additionalReplacements;

public CodeFix(string description, bool isPreferred, CodeReplacement replacement)
{
this.Description = description;
this.IsPreferred = isPreferred;
this.replacement = replacement;
this.additionalReplacements = null;
}

public CodeFix(string description, bool isPreferred, CodeReplacement replacement, params CodeReplacement[] additionalReplacement)
: this(description, isPreferred, replacement)
{
this.additionalReplacements = additionalReplacement;
}

public string Description { get; }

/// <summary>
/// Marks this as preferred. Only useful for the language server.
/// In VSCode, preferred actions are used by the "auto fix" command and can be targeted by keybindings.
/// </summary>
public bool IsPreferred { get; }

public IEnumerable<CodeReplacement> Replacements
{
get
{
yield return this.replacement;

if (this.additionalReplacements != null)
{
foreach (var replacement in this.additionalReplacements)
{
yield return replacement;
}
}
}
}
}
}
15 changes: 15 additions & 0 deletions src/Bicep.Core/CodeAction/CodeManipulator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using Bicep.Core.Parser;

namespace Bicep.Core.CodeAction
{
public static class CodeManipulator
{
public static CodeReplacement Replace(TextSpan span, string text)
=> new CodeReplacement(span, text);

public static CodeReplacement Replace(IPositionable positionable, string text)
=> Replace(positionable.Span, text);
}
}
19 changes: 19 additions & 0 deletions src/Bicep.Core/CodeAction/CodeReplacement.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using Bicep.Core.Parser;

namespace Bicep.Core.CodeAction
{
public class CodeReplacement : IPositionable
{
public CodeReplacement(TextSpan span, string text)
{
this.Span = span;
this.Text = text;
}

public TextSpan Span { get; }

public string Text { get; }
}
}
12 changes: 12 additions & 0 deletions src/Bicep.Core/CodeAction/IFixable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;
using Bicep.Core.Parser;

namespace Bicep.Core.CodeAction
{
public interface IFixable : IPositionable
{
public IEnumerable<CodeFix> Fixes { get; }
}
}
11 changes: 7 additions & 4 deletions src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Bicep.Core.Parser;
using Bicep.Core.TypeSystem;
using Bicep.Core.Resources;
using Bicep.Core.CodeAction;
using System.Linq;

namespace Bicep.Core.Diagnostics
Expand Down Expand Up @@ -447,16 +448,18 @@ public ErrorDiagnostic ArgumentCountMismatch(int argumentCount, int mininumArgum
"BCP081",
$"Resource type \"{resourceTypeReference.FormatName()}\" does not have types available.");

public ErrorDiagnostic SymbolicNameDoesNotExistWithSuggestion(string name, string suggestedName) => new ErrorDiagnostic(
public FixableErrorDiagnostic SymbolicNameDoesNotExistWithSuggestion(string name, string suggestedName) => new FixableErrorDiagnostic(
TextSpan,
"BCP082",
$"The name \"{name}\" does not exist in the current context. Did you mean \"{suggestedName}\"?");
$"The name \"{name}\" does not exist in the current context. Did you mean \"{suggestedName}\"?",
new CodeFix($"Change \"{name}\" to \"{suggestedName}\"", true, CodeManipulator.Replace(TextSpan, suggestedName)));

public Diagnostic UnknownPropertyWithSuggestion(bool warnInsteadOfError, TypeSymbol type, string badProperty, string suggestedProperty) => new Diagnostic(
public FixableDiagnostic UnknownPropertyWithSuggestion(bool warnInsteadOfError, TypeSymbol type, string badProperty, string suggestedProperty) => new FixableDiagnostic(
TextSpan,
warnInsteadOfError ? DiagnosticLevel.Warning : DiagnosticLevel.Error,
"BCP083",
$"The type \"{type}\" does not contain property \"{badProperty}\". Did you mean \"{suggestedProperty}\"?");
$"The type \"{type}\" does not contain property \"{badProperty}\". Did you mean \"{suggestedProperty}\"?",
new CodeFix($"Change \"{badProperty}\" to \"{suggestedProperty}\"", true, CodeManipulator.Replace(TextSpan, suggestedProperty)));

public ErrorDiagnostic SymbolicNameCannotUseReservedNamespaceName(string name, IEnumerable<string> namespaces) => new ErrorDiagnostic(
TextSpan,
Expand Down
42 changes: 42 additions & 0 deletions src/Bicep.Core/Diagnostics/FixableDiagnostic.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;
using Bicep.Core.CodeAction;

namespace Bicep.Core.Diagnostics
{
public class FixableDiagnostic : Diagnostic, IFixable
{
private readonly CodeFix fix;
private readonly IEnumerable<CodeFix>? additionalFixes;

public FixableDiagnostic(Parser.TextSpan span, DiagnosticLevel level, string code, string message, CodeFix fix)
: base(span, level, code, message)
{
this.fix = fix;
this.additionalFixes = null;
}

public FixableDiagnostic(Parser.TextSpan span, DiagnosticLevel level, string code, string message, CodeFix fix, params CodeFix[] additionalFixes)
: this(span, level, code, message, fix)
{
this.additionalFixes = additionalFixes;
}

public IEnumerable<CodeFix> Fixes
{
get
{
yield return this.fix;

if (this.additionalFixes != null)
{
foreach (var fix in this.additionalFixes)
{
yield return fix;
}
}
}
}
}
}
42 changes: 42 additions & 0 deletions src/Bicep.Core/Diagnostics/FixableErrorDiagnostic.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;
using Bicep.Core.CodeAction;

namespace Bicep.Core.Diagnostics
{
public class FixableErrorDiagnostic : ErrorDiagnostic, IFixable
{
private readonly CodeFix fix;
private readonly IEnumerable<CodeFix>? additionalFixes;

public FixableErrorDiagnostic(Parser.TextSpan span, string code, string message, CodeFix fix)
: base(span, code, message)
{
this.fix = fix;
this.additionalFixes = null;
}

public FixableErrorDiagnostic(Parser.TextSpan span, string code, string message, CodeFix fix, params CodeFix[] additionalFixes)
: this(span, code, message, fix)
{
this.additionalFixes = additionalFixes;
}

public IEnumerable<CodeFix> Fixes
{
get
{
yield return this.fix;

if (this.additionalFixes != null)
{
foreach (var fix in this.additionalFixes)
{
yield return fix;
}
}
}
}
}
}
4 changes: 2 additions & 2 deletions src/Bicep.Core/Parser/TextSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public TextSpan(int position, int length)

public bool Contains(int offset) => offset >= this.Position && offset < this.Position + this.Length;

public bool ContainsInclusive(int offset) => offset >= this.Position && offset <= this.Position + this.Length;

public bool ContainsInclusive(int offset) => offset >= this.Position && offset <= this.Position + this.Length;

/// <summary>
/// Calculates the span from the beginning of the first span to the end of the second span.
/// </summary>
Expand Down
19 changes: 10 additions & 9 deletions src/Bicep.Core/TypeSystem/TypeAssignmentVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -828,16 +828,17 @@ private static TypeSymbol GetNamedPropertyType(ObjectType baseType, IPositionabl
.OrderBy(x => x);

var diagnosticBuilder = DiagnosticBuilder.ForPosition(propertyExpressionPositionable);

var unknownPropertyDiagnostic = availableProperties.Any() switch
{
true => SpellChecker.GetSpellingSuggestion(propertyName, availableProperties) switch
{
string suggestedPropertyName
when suggestedPropertyName != null=> diagnosticBuilder.UnknownPropertyWithSuggestion(TypeValidator.ShouldWarn(baseType), baseType, propertyName, suggestedPropertyName),
_ => diagnosticBuilder.UnknownPropertyWithAvailableProperties(TypeValidator.ShouldWarn(baseType), baseType, propertyName, availableProperties),
var shouldWarn = TypeValidator.ShouldWarn(baseType);

var unknownPropertyDiagnostic = availableProperties.Any() switch
{
true => SpellChecker.GetSpellingSuggestion(propertyName, availableProperties) switch
{
string suggestedPropertyName when suggestedPropertyName != null =>
diagnosticBuilder.UnknownPropertyWithSuggestion(shouldWarn, baseType, propertyName, suggestedPropertyName),
_ => diagnosticBuilder.UnknownPropertyWithAvailableProperties(shouldWarn, baseType, propertyName, availableProperties),
},
_ => diagnosticBuilder.UnknownProperty(TypeValidator.ShouldWarn(baseType), baseType, propertyName)
_ => diagnosticBuilder.UnknownProperty(shouldWarn, baseType, propertyName)
};

diagnostics.Add(unknownPropertyDiagnostic);
Expand Down
Loading

0 comments on commit fe817ab

Please sign in to comment.