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

Added a new test project for c# 11 #3491

Merged
merged 5 commits into from May 2, 2022

Conversation

bjornhellander
Copy link
Contributor

Added test project for c# 11, to prepare for coming c# 11 tests.
(Copied c# 10 project, changed names and removed existing tests.)

@codecov
Copy link

codecov bot commented Apr 24, 2022

Codecov Report

Merging #3491 (6dc6195) into master (9c5d064) will decrease coverage by 0.00%.
The diff coverage is 90.78%.

@@            Coverage Diff             @@
##           master    #3491      +/-   ##
==========================================
- Coverage   93.25%   93.24%   -0.01%     
==========================================
  Files        1066     1066              
  Lines      113311   113343      +32     
  Branches     3998     4002       +4     
==========================================
+ Hits       105664   105690      +26     
- Misses       6619     6623       +4     
- Partials     1028     1030       +2     


public class SA1600CSharp11UnitTests : SA1600CSharp10UnitTests
{
protected override DiagnosticResult[] GetExpectedResultTestRegressionMethodGlobalNamespace(string code)
Copy link
Member

Choose a reason for hiding this comment

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

❔ Is this different from the base class?

Copy link
Member

Choose a reason for hiding this comment

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

I believe this override can be removed and the C# 11 test just use the one from C# 10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Comment on lines 6 to 14
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using StyleCop.Analyzers.Test.CSharp10.ReadabilityRules;
using StyleCop.Analyzers.Test.Verifiers;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.ReadabilityRules.SA1101PrefixLocalCallsWithThis,
StyleCop.Analyzers.ReadabilityRules.SA1101CodeFixProvider>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using StyleCop.Analyzers.Test.CSharp10.ReadabilityRules;
using StyleCop.Analyzers.Test.Verifiers;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.ReadabilityRules.SA1101PrefixLocalCallsWithThis,
StyleCop.Analyzers.ReadabilityRules.SA1101CodeFixProvider>;
using StyleCop.Analyzers.Test.CSharp10.ReadabilityRules;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.0.1" />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.0.1" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.2.0-2.final" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would require an update of Microsoft.CodeAnalysis.Analyzers to version 3.3.3, which in turn would require some code fixes. Not much, but still. Ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fixes would be due to RS1034 (Prefer 'IsKind' for checking syntax kinds)

Copy link
Member

Choose a reason for hiding this comment

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

That should be fine, especially if the update to 4.2.0-2.final occurs as its own commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

4.2.0-4.final arrived, so I used that one instead

@sharwell
Copy link
Member

sharwell commented Apr 27, 2022

📝 This change doesn't actually allow C# 11 features. You'll need updates to the following lines to use LanguageVersion.Preview instead of the default value null:

You'll want to use this default whenever LightupHelpers.SupportsCSharp11 is true, which is a new property defined right below this:

public static bool SupportsCSharp10 { get; }
= Enum.GetNames(typeof(LanguageVersion)).Contains(nameof(LanguageVersionEx.CSharp10));

@bjornhellander
Copy link
Contributor Author

Using a new version of Roslyn in the c# 11 tests also fixes #3492.

@sharwell sharwell merged commit e0ee7b2 into DotNetAnalyzers:master May 2, 2022
@bjornhellander bjornhellander deleted the feature/test11 branch May 2, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants