-
Notifications
You must be signed in to change notification settings - Fork 508
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
Add code fixes for documentation diagnostics (#1) #3445
base: master
Are you sure you want to change the base?
Conversation
* Add helpers * Add documenation fix SA1600 for most types * Add SA1611 codefix * Add SA1602 codefix * Add SA1618 codefix Co-authored-by: Alon Sheffer <[email protected]> Co-authored-by: Jonatan Gefen <[email protected]>
Style fixes
Codecov Report
@@ Coverage Diff @@
## master #3445 +/- ##
==========================================
- Coverage 93.17% 93.17% -0.01%
==========================================
Files 1047 1058 +11
Lines 112348 113868 +1520
Branches 3976 4039 +63
==========================================
+ Hits 104684 106100 +1416
- Misses 6644 6711 +67
- Partials 1020 1057 +37 |
📝 Keep in mind I'll be primarily evaluating this in the context of #764 (comment). It seems likely that you'll want one or more features that don't get merged here. For that, it's possible to add a custom code fix project which recognizes diagnostics produced by StyleCop Analyzers and can provide fixes beyond the scope of this project. |
enum TypeName | ||
{ | ||
/// <summary> | ||
/// Bar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 This isn't a meaningful addition, so the preference is to not automatically generate it. It's fine to generate the empty <summary>
element, but the contents should not be automatically generated except in limited cases where a specific design pattern includes the documentation form.
/// <summary> | ||
/// Foo | ||
/// </summary> | ||
/// <param name=""param1"">The param1.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 The content should not be added here, except in limited cases where the content is dictated by a specific design pattern.
/// <param name=""param1"">The param1.</param> | |
/// <param name=""param1""></param> |
/// Foo | ||
/// </summary> | ||
/// <param name=""param1"">Param 1</param> | ||
/// <param name=""param2"">The param2.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <param name=""param2"">The param2.</param> | |
/// <param name=""param2""></param> |
/// Foo | ||
/// </summary> | ||
/// <typeparam name=""TInternalRow"">The type of the internal row.</typeparam> | ||
/// <param name=""param1"">The param1.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <param name=""param1"">The param1.</param> | |
/// <param name=""param1""></param> |
/// </summary> | ||
/// <param name=""param1"">The param1.</param> | ||
/// <param name=""param2"">The param2.</param> | ||
/// <param name=""param3"">The param3.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <param name=""param3"">The param3.</param> | |
/// <param name=""param3""></param> |
/// Foo | ||
/// </summary> | ||
/// <typeparam name=""Ta"">The type of the ta.</typeparam> | ||
/// <typeparam name=""Tb"">The type of the tb.</typeparam> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <typeparam name=""Tb"">The type of the tb.</typeparam> | |
/// <typeparam name=""Tb""></typeparam> |
<ProjectReference | ||
Include="..\StyleCop.Analyzers.CodeGeneration\StyleCop.Analyzers.CodeGeneration.csproj" | ||
SetTargetFramework="TargetFramework=netstandard2.0" | ||
OutputItemType="Analyzer" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 I'm guessing the change here was unintentional?
/// <param name=""param1""></param> | ||
/// <param name=""param2""></param> | ||
/// <param name=""param1"">The param1.</param> | ||
/// <param name=""param2"">If true, param2.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 The standard form for a Boolean-value property is:
<see langword="true"/> if [text]; otherwise, <see langword="false"/>.
This could be generated using a placeholder:
<see langword="true"/> if <placeholder>condition</placeholder>; otherwise, <see langword="false"/>.
Diagnostic().WithLocation(9, 17), | ||
Diagnostic().WithLocation(13, 16), | ||
Diagnostic().WithLocation(18, 16), | ||
Diagnostic().WithLocation(23, 16), | ||
Diagnostic().WithLocation(28, 40), | ||
Diagnostic().WithLocation(33, 25), | ||
Diagnostic().WithLocation(38, 25), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Use [|...|]
syntax inside of testCode
to mark the diagnostic locations
/// <summary> | ||
/// Test class. | ||
/// </summary> | ||
public class TestClass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Most values added in this test need to be updated according to other comments given previously.
I'm looking through the older pull requests in this repo. Is this something that you will continue working on, @jgefen? |
Add helpers
Add documenation fix SA1600 for most types
Add SA1611 codefix
Add SA1602 codefix
Add SA1618 codefix
Co-authored-by: Alon Sheffer [email protected]
Co-authored-by: Jonatan Gefen [email protected]