Skip to content

Commit

Permalink
add: support [RequireNamedArgs] on extension methods
Browse files Browse the repository at this point in the history 
- https://github.com/kkkmail investigated and contributed code
  necessary to support `[RequireNamedArgs]` on extension methods.
  Details: #1

- Testing support code had to be updated to support the extension methods related tests

fixes #1
  • Loading branch information
@mykolav
mykolav committed Aug 23, 2020
1 parent 9345894 commit b4feb1c
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 58 deletions.
130 changes: 94 additions & 36 deletions RequireNamedArgs.Tests/Analyzer/AnalyzerTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,41 @@ module private Expect =
open Support.DiagnosticProvider
open Support.DocumentFactory

let formatSource (source: string) =
// Wraps `source` with a namespace
let ns (source: string) =
sprintf
"namespace Frobnitz
"using System;
namespace Frobnitz
{
class Wombat
{
%s
}
%s
[AttributeUsage(AttributeTargets.Method)]
class RequireNamedArgsAttribute : Attribute {}
}" source
}
class Program { static void Main(string[] args) {} }
" source


let toBeEmittedFrom snippet expectedDiags =
let code = formatSource snippet
let code = ns snippet
let analyzer = RequireNamedArgsAnalyzer()
expectedDiags
|> Expect.diagnosticsToMatch analyzer
(analyzer.GetSortedDiagnostics(CSharp, [code]))

let emptyDiagnostics snippet = [||] |> toBeEmittedFrom (formatSource snippet)

let emptyDiagnostics snippet = [||] |> toBeEmittedFrom snippet


// Wraps `source` with a class
let klass (source: string) =
sprintf
"class Wombat
{
%s
}" source


// TODO: Consider the following use-cases:
// TODO: Delegate. class C { void M(System.Action<int, int> f) => f(1, 2);
Expand All @@ -43,86 +56,131 @@ module private Expect =
let analyzerTests =
testList "The RequireNamedArgs analyzer tests" [
test "Empty code does not trigger diagnostics" {
Expect.emptyDiagnostics @"";
Expect.emptyDiagnostics (klass @"");
}
test "Method w/o params does not trigger diagnostics" {
Expect.emptyDiagnostics @"
Expect.emptyDiagnostics (klass @"
void Gork() {}
void Bork() { Gork(); }
"
")
}
test "Method w/o params w/ [RequireNamedArgs] does not trigger diagnostics" {
Expect.emptyDiagnostics @"
Expect.emptyDiagnostics (klass @"
[RequireNamedArgs]
void Gork() {}
void Bork() { Gork(); }
"
")
}
test "Method w/o [RequireNamedArgs] does not trigger diagnostics" {
Expect.emptyDiagnostics @"
class Wombat
{
void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(name: ""Goku"", powerLevel: 9001); }
}
"
Expect.emptyDiagnostics (klass @"
void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(name: ""Goku"", powerLevel: 9001); }
")
}
test "Method w/ [RequireNamedArgs] invoked w/ named args does not trigger diagnostics" {
Expect.emptyDiagnostics @"
Expect.emptyDiagnostics (klass @"
[RequireNamedArgs]
void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(name: ""Goku"", powerLevel: 9001); }
"
")
}
test "Method w/ [RequireNamedArgs] invoked w/ positional args triggers diagnostic" {
let testCodeSnippet = @"
let testCodeSnippet = (klass @"
[RequireNamedArgs]
void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(""Goku"", 9001); }
"
")

let expectedDiag = RequireNamedArgsDiagResult.Create(invokedMethod="TellPowerLevel",
paramNamesByType=[[ "line"; "column" ]],
fileName="Test0.cs", line=8u, column=31u)
fileName="Test0.cs", line=9u, column=31u)

[|expectedDiag|] |> Expect.toBeEmittedFrom testCodeSnippet
}
test "Method w/ [RequireNamedArgs] attribute invoked w/ positional args triggers diagnostic" {
let testCodeSnippet = @"
let testCodeSnippet = (klass @"
[RequireNamedArgs]
void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(""Goku"", 9001); }
"
")

let expectedDiag = RequireNamedArgsDiagResult.Create(invokedMethod="TellPowerLevel",
paramNamesByType=[[ "line"; "column" ]],
fileName="Test0.cs", line=8u, column=31u)
fileName="Test0.cs", line=9u, column=31u)

[|expectedDiag|] |> Expect.toBeEmittedFrom testCodeSnippet
}
test "Static method w/ [RequireNamedArgs] invoked w/ positional args triggers diagnostic" {
let testCodeSnippet = @"
let testCodeSnippet = (klass @"
[RequireNamedArgs]
static void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(""Goku"", 9001); }
"
")

let expectedDiag = RequireNamedArgsDiagResult.Create(invokedMethod="TellPowerLevel",
paramNamesByType=[[ "line"; "column" ]],
fileName="Test0.cs", line=8u, column=31u)
fileName="Test0.cs", line=9u, column=31u)

[|expectedDiag|] |> Expect.toBeEmittedFrom testCodeSnippet
}
test "Private static method w/ [RequireNamedArgs] invoked w/ positional args triggers diagnostic" {
let testCodeSnippet = @"
let testCodeSnippet = (klass @"
[RequireNamedArgs]
private static void TellPowerLevel(string name, int powerLevel) {}
void Bork() { TellPowerLevel(""Goku"", 9001); }
"
")

let expectedDiag = RequireNamedArgsDiagResult.Create(invokedMethod="TellPowerLevel",
paramNamesByType=[[ "line"; "column" ]],
fileName="Test0.cs", line=8u, column=31u)
fileName="Test0.cs", line=9u, column=31u)

[|expectedDiag|] |> Expect.toBeEmittedFrom testCodeSnippet
} ]
}
// See https://github.com/mykolav/require-named-args-fs/issues/1
test "Extension method w/o [RequireNamedArgs] does not triggers diagnostic" {
Expect.emptyDiagnostics @"
static class PowerLevelExtensions
{
public static void SwitchPowerLevel(this string name, Action onOver9000, Action onUnderOrAt9000) {}
}
class Wombat
{
void Bork() { ""Goku"".SwitchPowerLevel(() => {}, () => {}); }
}
"
}
test "Extension method w/ [RequireNamedArgs] invoked w/ names args does not trigger diagnostic" {
Expect.emptyDiagnostics @"
static class PowerLevelExtensions
{
[RequireNamedArgs]
public static void SwitchPowerLevel(this string name, Action onOver9000, Action onUnderOrAt9000) {}
}
class Wombat
{
void Bork() { ""Goku"".SwitchPowerLevel(onOver9000: () => {}, onUnderOrAt9000: () => {}); }
}
"
}
test "Extension method w/ [RequireNamedArgs] invoked w/ positional args triggers diagnostic" {
let testCodeSnippet = @"
static class PowerLevelExtensions
{
[RequireNamedArgs]
public static void SwitchPowerLevel(this string name, Action onOver9000, Action onUnderOrAt9000) {}
}
class Wombat
{
void Bork() { ""Goku"".SwitchPowerLevel(() => {}, () => {}); }
}
"

let expectedDiag = RequireNamedArgsDiagResult.Create(invokedMethod="SwitchPowerLevel",
paramNamesByType=[[ "line"; "column" ]],
fileName="Test0.cs", line=13u, column=35u)

[| expectedDiag |] |> Expect.toBeEmittedFrom testCodeSnippet
}]
9 changes: 6 additions & 3 deletions RequireNamedArgs.Tests/CodeFix/CodeFixProviderTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ open RequireNamedArgs.Tests.Support.DocumentFactory
module private Expect =
let formatSource (source: string) =
sprintf
"namespace Frobnitz
"using System;
namespace Frobnitz
{
class Wombat
{
%s
}
[AttributeUsage(AttributeTargets.Method)]
class RequireNamedArgsAttribute : Attribute {}
class RequireNamedArgsAttribute : Attribute {}
class Program { static void Main(string[] args) {} }
}" source

let toBeFixedAndMatch expectedFixedSnippet originalSnippet =
Expand Down Expand Up @@ -161,5 +164,5 @@ let codeFixProviderTests =
"

originalSnippet |> Expect.toBeFixedAndMatch expectedFixedSnippet
} ]
}]
]
37 changes: 27 additions & 10 deletions RequireNamedArgs.Tests/Support/DiagnosticProvider.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,31 @@ type DiagnosticAnalyzer with
else documents
|> Seq.exists (fun doc -> doc.GetSyntaxTreeAsync().Result = loc.SourceTree)

let sortedDiags = documents
|> Seq.map (fun doc -> doc.Project)
|> Seq.distinct
|> Seq.map (fun proj -> proj.GetCompilationAsync().Result
.WithAnalyzers(ImmutableArray.Create(analyzer)))
|> Seq.collect (fun compilation -> compilation.GetAnalyzerDiagnosticsAsync()
.Result)
|> Seq.filter shouldTake
|> Seq.sortBy (fun diag -> diag.Location.SourceSpan.Start)
|> List.ofSeq
let compilations =
documents
|> Seq.map (fun doc -> doc.Project)
|> Seq.distinct
|> Seq.map (fun proj -> proj.GetCompilationAsync().Result)

// For details, see https://stackoverflow.com/a/54129600/818321
let compilationErrors =
compilations
|> Seq.collect (fun c -> c.GetDiagnostics() |> Seq.where (fun d -> d.Severity = DiagnosticSeverity.Error))
|> Seq.sortBy (fun d -> d.Location.SourceSpan.Start)
|> List.ofSeq

if compilationErrors.Length > 0
then
compilationErrors
else

let sortedDiags =
compilations
|> Seq.map (fun compilation -> compilation.WithAnalyzers(ImmutableArray.Create(analyzer)))
|> Seq.collect (fun compilation -> compilation.GetAnalyzerDiagnosticsAsync()
.Result)
|> Seq.filter shouldTake
|> Seq.sortBy (fun diag -> diag.Location.SourceSpan.Start)
|> List.ofSeq

sortedDiags
7 changes: 7 additions & 0 deletions RequireNamedArgs.Tests/Support/DocumentFactory.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ open System.Linq
open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.CSharp
open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.VisualBasic

let DefaultFilePathPrefix = "Test";
let CSharpDefaultFileExt = "cs";
Expand Down Expand Up @@ -35,9 +36,15 @@ let private mkProject(sources: seq<string>, lang: Langs) =

let projId = ProjectId.CreateNewId(debugName=TestProjectName)

let parseOptions =
if lang = Langs.CSharp
then CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_2) :> ParseOptions
else VisualBasicParseOptions.Default :> ParseOptions

let solution = (new AdhocWorkspace())
.CurrentSolution
.AddProject(projId, TestProjectName, TestProjectName, lang.ToString())
.WithProjectParseOptions(projId, parseOptions)
.AddMetadataReference(projId, CorlibRef)
.AddMetadataReference(projId, SystemCoreRef)
.AddMetadataReference(projId, CSharpSymbolsRef)
Expand Down
6 changes: 3 additions & 3 deletions RequireNamedArgs/RequireNamedArgs.nuspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
<package >
<metadata>
<id>RequireNamedArgs</id>
<version>0.0.2</version>
<version>0.0.3</version>
<authors>Mykola Musiienko</authors>
<owners>Mykola Musiienko</owners>
<requireLicenseAcceptance>false</requireLicenseAcceptance>
<license type="file">LICENSE</license>
<projectUrl>https://github.com/mykolav/require-named-args-fs</projectUrl>
<icon>icon.png</icon>
<description>"Require named arguments" Roslyn code analyzer and code-fix provider for C#</description>
<copyright>Copyright © Mykola Musiienko 2019</copyright>
<copyright>Copyright © Mykola Musiienko 2020</copyright>
<tags>roslyn roslyn-analyzer</tags>
<repository url="https://github.com/mykolav/require-named-args-fs" />
<releaseNotes>Look for method definitions marked with a `[RequireNamedArgs]` attribute instead of a special comment</releaseNotes>
<releaseNotes>https://github.com/kkkmail contributed support for extension methods</releaseNotes>

<dependencies>
<group>
Expand Down
13 changes: 7 additions & 6 deletions RequireNamedArgs/RequireNamedArgsAnalyzer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ open RequireNamedArgs.CSharpAdapters
open RequireNamedArgs.InvocationExprSyntax
open RequireNamedArgs.MaybeBuilder

[<DiagnosticAnalyzer(Microsoft.CodeAnalysis.LanguageNames.CSharp)>]
[<DiagnosticAnalyzer(LanguageNames.CSharp)>]
type public RequireNamedArgsAnalyzer() =
inherit DiagnosticAnalyzer()

Expand Down Expand Up @@ -45,11 +45,12 @@ type public RequireNamedArgsAnalyzer() =

member private this.filterSupported (methodSymbol: IMethodSymbol) =
match methodSymbol.MethodKind with
// So far we only support analyzing of the three kinds of methods listed below.
| MethodKind.Ordinary
| MethodKind.Constructor
| MethodKind.LocalFunction -> Some methodSymbol
| _ -> None
// So far we only support analyzing of the four kinds of methods listed below.
| MethodKind.Ordinary
| MethodKind.Constructor
| MethodKind.LocalFunction
| MethodKind.ReducedExtension -> Some methodSymbol
| _ -> None

member private this.formatDiagMessage argsWhichShouldBeNamed =
String.Join(
Expand Down

0 comments on commit b4feb1c

Please sign in to comment.