Skip to content

Add skipped failing test for IDE0044#45288

Merged
mavasani merged 23 commits intodotnet:masterfrom
Youssef1313:patch-1
Aug 31, 2020
Merged

Add skipped failing test for IDE0044#45288
mavasani merged 23 commits intodotnet:masterfrom
Youssef1313:patch-1

Conversation

@Youssef1313
Copy link
Member

The linked issue is now closed and has Resolution-Fixed label. So, this false positive should have been fixed.

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 18, 2020 11:42
@mavasani
Copy link
Contributor

@Youssef1313 Seems like IDE0044 is still firing here.

@Youssef1313
Copy link
Member Author

Then the issue should be re-opened?
Pinging @sharwell as he worked on that issue.

@mavasani
Copy link
Contributor

@Youssef1313 The fix was made in the CodeStyle analyzer NuGet and we need to move to a newer CodeStyle package to get the fix.

@mavasani
Copy link
Contributor

@Youssef1313
Copy link
Member Author

Aha, got it. Are there any reasons not to update?

@mavasani
Copy link
Contributor

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 18, 2020 15:36
@mavasani
Copy link
Contributor

@Youssef1313 eeba605 seems to be downgrading the version. Can you please upgrade to latest 3.8.x at https://dotnet.myget.org/feed/roslyn/package/nuget/Microsoft.CodeAnalysis.CSharp.CodeStyle version?

@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jun 19, 2020
@Youssef1313
Copy link
Member Author

@mavasani The failure is still there with the 3.8.x version. What could be the issue?

@sharwell
Copy link
Contributor

Updating the code style package is a bit complicated. The dotnet-format version needs to be updated at the same time to a version built from the same source, but the version numbers themselves will be different. I can check with @JoeRobich next week to see what the most recent matching pair is.

@Youssef1313
Copy link
Member Author

Using the latest dotnet-format version doesn't seem to work. I'll wait until you check the latest matching pair.

@Youssef1313
Copy link
Member Author

Ping @sharwell

@sharwell
Copy link
Contributor

@JoeRobich can you verify the matching versions here?

@JoeRobich
Copy link
Member

@JoeRobich can you verify the matching versions here?

Currently dotnet-format is tracking the VS 16.7 feed, so to generate a matching version I'll need to update to the 16.8 feed and trigger an update. It will pull in the most recent 3.8.0 build. I'll reply back when a build is ready with dotnet-format version and the M.CA version used.

@JoeRobich
Copy link
Member

@sharwell @Youssef1313 After updating dotnet-format to consume 3.8.0 builds, here is the matched pair.

Microsoft.CodeAnalysis - 3.8.0-1.20367.11
dotnet-format - 5.0.137002

@Youssef1313
Copy link
Member Author

@sharwell @JoeRobich The build is still failing.

@Youssef1313
Copy link
Member Author

ping @sharwell @JoeRobich

@JoeRobich
Copy link
Member

But I'll need @JoeRobich help with conflicts regarding the versions.

@Youssef1313 If your changes don't rely on updated dependencies then I would use the versions from master.

@Youssef1313 Youssef1313 changed the title Remove IDE0044 suppression Fix IDE0055, Add skipped failing test for IDE0044 Aug 13, 2020
@Youssef1313
Copy link
Member Author

I reverted all version updates in this PR, and skipped the new failing test.
@mavasani, the issue can now be re-opened to address the skipped test.

@Youssef1313 Youssef1313 changed the title Fix IDE0055, Add skipped failing test for IDE0044 Add skipped failing test for IDE0044 Aug 26, 2020
@Youssef1313 Youssef1313 marked this pull request as draft August 26, 2020 21:39
@Youssef1313 Youssef1313 marked this pull request as ready for review August 26, 2020 21:40
@Youssef1313
Copy link
Member Author

Please squash when merging.

@333fred
Copy link
Member

333fred commented Aug 27, 2020

@mavasani @sharwell what is this waiting on?

@Youssef1313 Youssef1313 marked this pull request as draft August 27, 2020 19:41
@Youssef1313 Youssef1313 marked this pull request as ready for review August 27, 2020 19:41
@sharwell
Copy link
Contributor

@333fred it should be ready to manually merge (with a squash commit per #45288 (comment)) when the build completes

@mavasani mavasani merged commit e7bad77 into dotnet:master Aug 31, 2020
@ghost ghost added this to the Next milestone Aug 31, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Aug 31, 2020
* upstream/master: (43 commits)
  Fix typos (dotnet#47264)
  Disable CS0660 where Full Solution Analysis produces a different result
  Pass in correct arguments to TypeScript handler
  Add skipped failing test for IDE0044 (dotnet#45288)
  Avoid first chance exception
  Code review feedback part 3
  Update stale URLs in the readme
  Update IntegrationTestSourceGenerator.cs
  Fix comment
  Update IntegrationTestSourceGenerator.cs
  Remove unused reference to obsolete class
  Update nullable annotations in Classification folder
  Update generator public API names: (dotnet#47222)
  NullableContextAttribute for property parameters is from accessor (dotnet#47223)
  Modify global analyzer config precedence: (dotnet#45871)
  Skip the C# source generator integration test running for now
  Additional XAML LSP handlers (dotnet#47217)
  Remap diagnostics using IWorkspaceVenusSpanMappingService in-proc
  Warn on type named "record" (dotnet#47094)
  Bail out early before getting SyntaxTree
  ...
@allisonchou allisonchou modified the milestones: Next, 16.8.P3 Aug 31, 2020
@sharwell sharwell added the Feature - IDE0044 Make field readonly label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - IDE0044 Make field readonly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants