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

SA 1642 Is Triggered with Private Constructor Text. #1318

Closed
lumberjack4 opened this issue Aug 29, 2015 · 3 comments
Closed

SA 1642 Is Triggered with Private Constructor Text. #1318

lumberjack4 opened this issue Aug 29, 2015 · 3 comments
Assignees
Milestone

Comments

@lumberjack4
Copy link

/// <summary>
/// Prevents a default instance of the <see cref="MyClass" /> class from being created externally.
/// </summary>
private MyClass()
{
}

Triggers SA 1642. Code fix attempts to replace the summary with the public constructor default text. This was seen using beta 8.

@sharwell
Copy link
Member

You are seeing two different things here.

  1. SA1642 is being reported because you didn't end your sentence following the word created. The analyzer currently expected created. (with the .).
  2. The replacement text is an intentional deviation from StyleCop "classic" (SA1642 (summary text) misleading for private constructors #413). SA1642 supports either wording for private constructors, and the code fix prefers the wording you saw being inserted.

My recommendation is updating your constructors to use the non-misleading public constructor wording.

@pdelvo Should we drop the . from the expected text in the analyzer?

@lumberjack4
Copy link
Author

I like the idea of leaving off the trailing . This appears to be the behavior for the normal public constructor text as most of my constructor documentation has more than just the starting text sequence and none of them trigger the diagnostic.

@sharwell
Copy link
Member

Keep in mind that your current constructor text is misleading. From outside the class, the constructor is not visible at all, which means it's not showing up in IntelliSense and you can't see the summary. The only place where the summary is visible is within the context of your current class, and in that scope it does not prevent the creation of new instances of the class.

  • If you want to prevent creation of instances of a class, use a static class.
  • If you want to allow creation of instances of your class, document the functionality it does do instead of the functionality it doesn't, which means use the "Initializes a new instance..." form even when the constructor is private.

📝 We may yet drop the . requirement. The above is just some suggestions for your information. 😄

@sharwell sharwell added this to the 1.0.0 Beta 9 milestone Aug 29, 2015
sharwell added a commit to sharwell/StyleCopAnalyzers that referenced this issue Aug 29, 2015
@sharwell sharwell self-assigned this Aug 29, 2015
sharwell added a commit to sharwell/StyleCopAnalyzers that referenced this issue Aug 29, 2015
sharwell added a commit to sharwell/StyleCopAnalyzers that referenced this issue Aug 29, 2015
sharwell added a commit to sharwell/StyleCopAnalyzers that referenced this issue Aug 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants