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 unit tests for C# diagnostic analyzers #86434

Closed
wants to merge 25 commits into from

Conversation

avilches
Copy link
Contributor

@avilches avilches commented Dec 22, 2023

This PR adds test cases for the analyzers in the Godot.SourceGenerators.Tests project, following the same spirit as @paulloz did on #82955.

  • MustBeVariantAnalyzer
  • GlobalClassAnalyzer

avilches and others added 19 commits December 8, 2023 01:17
…l-with-generic-typed-attributes' into fix-must-be-variant-analyzer-fail-with-generic-typed-attributes
…eVariantAnalyzer.cs

Co-authored-by: Raul Santos <raulsntos@gmail.com>
…eVariantAnalyzer.cs

Co-authored-by: Raul Santos <raulsntos@gmail.com>
@avilches avilches marked this pull request as ready for review December 22, 2023 15:04
@avilches avilches requested a review from a team as a code owner December 22, 2023 15:04
…-be-variant-tests

# Conflicts:
#	modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0401.cs
Comment on lines +14 to +16
new DiagnosticResult("GD0301", DiagnosticSeverity.Error)
.WithSpan(MustBeVariantGD0301, 11, 16, 11, 22)
.WithArguments(MustBeVariantGD0301)
Copy link
Member

Choose a reason for hiding this comment

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

Adding the expected errors here is a bit difficult to visualize, I'd prefer inlining it in the source if possible like the syntax described in Tutorial: Write your first analyzer and code fix:

public void MethodCallsError()
{
    // object is not Variant and Method<T> requires a generic type
    {|GD0301:Method<object>();|}
}

Copy link
Contributor Author

@avilches avilches Dec 25, 2023

Choose a reason for hiding this comment

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

I squashed the commits but I deleted the original branch in the process, as you can see in the history log... A new branch with exactly the same name and one single commit (with the {|GD0301:...|} change requested) has been created. Not sure if this PR could be reopened again or I should create a new one from the same branch again, just let me know. Sorry for the mess! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I squashed the commits but I deleted the original branch in the process, as you can see in the history log... A new branch with exactly the same name and one single commit (with the {|GD0301:...|} change requested) has been created. Not sure if this PR could be reopened again or I should create a new one from the same branch again, just let me know. Sorry for the mess! :)

Hi @AThousandShips should I create a new PR? or could you please reopen this one with the same branch, which is already restored?

Copy link
Member

Choose a reason for hiding this comment

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

I can't reopen this as the branch is deleted, maybe you can push to it again but otherwise I can't do anything here 🙂

I think a deleted branch can't be reopened in the system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a new one. Sorry for the mess: #86528

avilches and others added 2 commits December 22, 2023 18:26
…/TestData/Sources/MustBeVariant.GD0301.cs

Co-authored-by: Raul Santos <raulsntos@gmail.com>
…/TestData/Sources/MustBeVariant.GD0301.cs

Co-authored-by: Raul Santos <raulsntos@gmail.com>
@avilches avilches closed this Dec 25, 2023
@avilches avilches deleted the must-be-variant-tests branch December 25, 2023 22:20
@AThousandShips AThousandShips removed this from the 4.x milestone Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants