Update analyzer testing to Microsoft.CodeAnalysis.Testing #1272

sharwell · 2019-04-29T17:02:12Z

Description

Update the analyzer test project to use Microsoft.CodeAnalysis.Testing, the new shared analyzer testing library
Update the analyzer test project references to meet the minimum requirements of Microsoft.CodeAnalysis.Testing (no impact on end users)

Motivation and Context

This change makes it easier to comprehensively test analyzers.

Recommended review strategy: review each commit individually in sequence.

Testing

This is a test-only change.

Screenshots (if appropriate)

N/A

Types of changes

This is a test-only change; no observable impact on shipping code.

Checklist

My code follows the code style of this project
My change requires a change to the documentation
I have updated the documentation accordingly
I have read the README document
I have added tests to cover my changes
All new and existing tests passed

License

I confirm that this pull request can be released under the Apache 2 license

normj · 2019-08-13T06:48:54Z

I'm uncomfortable adding a myget feed to our build dependency. Is that required?

sharwell · 2019-08-13T17:19:57Z

@normj Yes, this is one of the primary locations where Roslyn team publishes intermediate packages. They are still signed binaries, but don't fall under release API compat guarantees. There is no immediate plan to publish the testing library to NuGet, despite the fact that it already works very well. We are still making changes to the API as we find new cases that people want to test.

sharwell · 2019-08-13T17:25:17Z

Please feel free to ask questions about anything you see. I try to answer questions directly, from a "this is how we arrived at the current state" view, as opposed to trying to say whether something is right or wrong. The testing library was developed to help people write better tests that in turn lead to better analyzers and better code fixes. We're interested in any feedback related to that end-to-end story so the community can be successful.

github-actions · 2022-02-02T00:09:29Z

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

sharwell · 2022-02-02T01:15:04Z

📝 The feed location changed; I can update this pull request to use the new one.

All analyzers in this package report diagnostics in both user and generated code.

sharwell · 2022-02-02T04:32:06Z

@normj This pull request now references code from nuget.org.

sharwell · 2022-02-02T04:32:54Z

sdk/code-analysis/AnalyzerUnitTests/AnalyzerUnitTests.csproj

@@ -36,32 +36,25 @@
  </PropertyGroup>
  <ItemGroup>
    <Reference Include="Microsoft.CodeAnalysis, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
-      <HintPath>..\..\packages\Microsoft.CodeAnalysis.Common.1.0.0\lib\net45\Microsoft.CodeAnalysis.dll</HintPath>
-      <Private>True</Private>
+      <HintPath>..\..\packages\Microsoft.CodeAnalysis.Common.1.0.1\lib\net45\Microsoft.CodeAnalysis.dll</HintPath>


📝 The changes in this commit would be tremendously simplified for the future if the project transitions to <PackageReference> for package references.

sharwell · 2022-02-02T04:34:18Z

sdk/code-analysis/AnalyzerUnitTests/CSharpCodeFixVerifier`2+Test.cs

@@ -22,6 +23,8 @@ public Test()

                    return solution;
                });
+
+                TestBehaviors |= TestBehaviors.SkipGeneratedCodeCheck;


📝 This line can be removed if the analyzers update to Roslyn 1.2.1 and add a call to ConfigureGeneratedCodeAnalysis.

ashishdhingra · 2023-03-20T21:55:23Z

@sharwell Good afternoon. I was reviewing outstanding PR(s) and came across this feature request. Please let me know if this PR is still relevant, or else confirm to close the same.

Thanks,
Ashish

sharwell · 2023-03-21T01:07:07Z

Hi @ashishdhingra ,

This change makes it easier to write tests for the analyzers in this project. Since it is a maintainability improvement and not an end-user product improvement, the relevance depends on the goals of the project. If the maintainers of this project intend to continue developing features in this repository, this change would still be relevant.

Thanks,
Sam

dscpinheiro · 2023-07-03T21:35:49Z

I'm closing this PR since we recently updated all of our analyzer projects to target netstandard2.0.

We're now using a much more modern version of Microsoft.CodeAnalysis (4.6.0 - https://github.com/aws/aws-sdk-net/blob/master/sdk/code-analysis/MockAnalyzer/MockAnalyzer.csproj#L9), and we also addressed some of the comments you mentioned (such as configuring generated code analysis - https://github.com/aws/aws-sdk-net/blob/master/sdk/code-analysis/SharedAnalysisCode/AbstractPropertyValueAssignmentAnalyzer.cs#L177-L178).

Please feel free to open a new issue if needed.

sharwell · 2023-07-05T18:46:40Z

Hi @dscpinheiro,

It appears the changes you describe were reverted. Please feel free to reach out if you have questions about updating this code.

Thanks,
Sam

dscpinheiro · 2023-07-05T20:05:57Z

Hi @sharwell! Yes, we ended up reverting because my change to the CodeAnalysis package broke customers.

We still want to update our analyzers to target netstandard2.0, and there was a recommendation on the other GitHub issue to target an older version (such as 2.10.0 - chosen from https://learn.microsoft.com/en-us/visualstudio/extensibility/roslyn-version-support?view=vs-2022 since it was the last one for VS2017).

Are there any concerns with this approach? We decided to revert the commit first (instead of only downgrading the package) as we weren't sure if that specific version could still break customers with older tooling (such as VS 2017 users prior to 15.9).

sharwell · 2023-07-06T15:20:42Z

We still want to update our analyzers to target netstandard2.0

Is there a specific motivating need to do this? Analyzers targeting netstandard1.3 (e.g. StyleCop Analyzers) will still execute within current versions of Visual Studio.

The 2.10.0 release of Microsoft.CodeAnalysis.Common targets netstandard1.3. I'm not sure what the official support status is for a netstandard2.0 analyzer is when running inside Visual Studio 2017. Most users I know of have either dropped VS 2017 support, or maintained the analyzers targeting netstandard1.3.

dscpinheiro · 2023-07-18T13:51:52Z

@sharwell We ended up targeting netstandard2.0: #2998

Thanks a lot for your help!

github-actions bot added the no-pr-activity label Feb 2, 2022

sharwell added 11 commits February 1, 2022 17:23

Update to MSTest packages from NuGet

17c4d53

Update to Microsoft.CodeAnalysis 1.0.1

250bdb8

Install Microsoft.CodeAnalysis.Testing 1.1.0

d65f3dc

Add project-specific verifier types

0867e22

Expose diagnostic descriptors to test code

9366bfc

Convert PropertyValueTests to Microsoft.CodeAnalysis.Testing

2b6d189

Declare compiler errors in analyzer test code

aa06d38

Remove unused helpers

500a726

Declare expected diagnostic spans instead of locations

04e6356

Fix accidental syntax errors in analyzer test code

ecbccc5

Skip the generated code check by default

b9aa097

All analyzers in this package report diagnostics in both user and generated code.

sharwell force-pushed the analyzer-testing branch from 117086b to b9aa097 Compare February 2, 2022 04:31

sharwell commented Feb 2, 2022

View reviewed changes

ashishdhingra added the feature-request A feature should be added or improved. label Oct 11, 2022

ashishdhingra removed the no-pr-activity label Mar 20, 2023

ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 20, 2023

github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 22, 2023

boblodgett assigned normj Jun 13, 2023

dscpinheiro closed this Jul 3, 2023

sharwell deleted the analyzer-testing branch July 25, 2023 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update analyzer testing to Microsoft.CodeAnalysis.Testing #1272

Update analyzer testing to Microsoft.CodeAnalysis.Testing #1272

sharwell commented Apr 29, 2019 •

edited

normj commented Aug 13, 2019

sharwell commented Aug 13, 2019 •

edited

sharwell commented Aug 13, 2019

github-actions bot commented Feb 2, 2022

sharwell commented Feb 2, 2022

sharwell commented Feb 2, 2022

sharwell Feb 2, 2022

sharwell Feb 2, 2022

ashishdhingra commented Mar 20, 2023

sharwell commented Mar 21, 2023

dscpinheiro commented Jul 3, 2023

sharwell commented Jul 5, 2023

dscpinheiro commented Jul 5, 2023 •

edited

sharwell commented Jul 6, 2023

dscpinheiro commented Jul 18, 2023

Update analyzer testing to Microsoft.CodeAnalysis.Testing #1272

Update analyzer testing to Microsoft.CodeAnalysis.Testing #1272

Conversation

sharwell commented Apr 29, 2019 • edited

Description

Motivation and Context

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

normj commented Aug 13, 2019

sharwell commented Aug 13, 2019 • edited

sharwell commented Aug 13, 2019

github-actions bot commented Feb 2, 2022

sharwell commented Feb 2, 2022

sharwell commented Feb 2, 2022

sharwell Feb 2, 2022

Choose a reason for hiding this comment

sharwell Feb 2, 2022

Choose a reason for hiding this comment

ashishdhingra commented Mar 20, 2023

sharwell commented Mar 21, 2023

dscpinheiro commented Jul 3, 2023

sharwell commented Jul 5, 2023

dscpinheiro commented Jul 5, 2023 • edited

sharwell commented Jul 6, 2023

dscpinheiro commented Jul 18, 2023

sharwell commented Apr 29, 2019 •

edited

sharwell commented Aug 13, 2019 •

edited

dscpinheiro commented Jul 5, 2023 •

edited