Skip to content

Fix the message for Make Field Readonly#26718

Merged
jinujoseph merged 2 commits intodotnet:masterfrom
sharwell:fix-resource
May 9, 2018
Merged

Fix the message for Make Field Readonly#26718
jinujoseph merged 2 commits intodotnet:masterfrom
sharwell:fix-resource

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

@sharwell sharwell commented May 8, 2018

Fixes #26219

Customer scenario

A customer upgrades to version 15.7. IDE0044 warnings start appearing, but no description is given for the warning so it's not clear to users what the problem is.

Bugs this fixes

Fixes #26219

Workarounds, if any

None, but also none needed. In order to understand a workaround, users would need to understand what IDE0044 is. Once a user understands what IDE0044 is without a description, no workaround is needed.

Risk

Very low. The fix is production code is clear and concise.

Performance impact

None.

Is this a regression from a previous update?

IDE0044 is new for the 15.7 release, so "errors appear without a description" could be considered a regression. On the other hand, IDE0044 never shipped in a previous release with a description.

Root cause analysis

Several items contributed to this issue:

  • No generalized test was included for this behavior
  • The IDE code is intentionally resilient to this type of failure, opting to omit missing text rather than fail (crash the IDE, etc.) because third-party extension developers can provide implementations of the same functionality

How was the bug found?

Customer reported

Test documentation updated?

No, but a regression test is added to ensure the situation does not reappear for existing or new diagnostics.

@sharwell sharwell requested a review from a team as a code owner May 8, 2018 12:01
@sharwell
Copy link
Copy Markdown
Contributor Author

sharwell commented May 8, 2018

@jinujoseph This is based on dev15.7.x branch so we can take it wherever needed.

}
catch (NotSupportedException)
{
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when does this happen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

➡️

Friend Overrides Function CreateDiagnosticProviderAndFixer(workspace As Workspace) As (DiagnosticAnalyzer, CodeFixProvider)
Throw New NotSupportedException()
End Function

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A comment might be wise here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

➡️ Fixed in 15f7469

public static DiagnosticDescriptor CreateSimpleDescriptor(string id)
{
return new DiagnosticDescriptor(id, title: "", messageFormat: "", category: "",
return new DiagnosticDescriptor(id, title: "", messageFormat: id, category: "",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this appropriate? seems... strange...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

💭 It's easier than further work to specialize the behavior for a test-only analyzer.

@sharwell sharwell added this to the 15.8 milestone May 8, 2018
}
catch (NotSupportedException)
{
return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A comment might be wise here.

@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge for 15.8.Preview2

@jinujoseph jinujoseph merged commit 2e69542 into dotnet:master May 9, 2018
@sharwell sharwell deleted the fix-resource branch May 9, 2018 18:41
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Readonly doesn't have a description for the Error List

5 participants