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

SA1005 Seems to get caught up on XML Docs #1012

Closed
jjmartin opened this issue Jul 20, 2015 · 19 comments
Closed

SA1005 Seems to get caught up on XML Docs #1012

jjmartin opened this issue Jul 20, 2015 · 19 comments
Assignees
Milestone

Comments

@jjmartin
Copy link

I'm getting a ton of SA1005 all pointing to the /// of a XML documentation

@pdelvo
Copy link
Member

pdelvo commented Jul 20, 2015

Thats weird. documentation trivia and comment trivia are syntactically completely different. Do you have an example?

@jjmartin
Copy link
Author

It took a while for them to show up again - but this is after restarting VS and then running code analysis.
I've filtered out everything but the SA1005. I'm not sure if its every XML doc we have in the solution - but its a lot.

image

@sharwell
Copy link
Member

@jjmartin By unfortunate coincidence (given that there are 10,000 violations reported), the file which is open in that screenshot isn't one of the files with warnings.

@sharwell
Copy link
Member

@jjmartin Can you add an example of a case where the warning is reported? So far I haven't seen it appear, and I've tried to reproduce it in multiple extensively-documented projects.

@sharwell sharwell added the bug label Jul 21, 2015
@FutureGUIs
Copy link

I'm getting the same experience now with the beta version in VS 2015 RTM.

@sharwell
Copy link
Member

@FutureGUIs Can you include an example piece of code which demonstrates the issue?

Thanks! 😄

@FutureGUIs
Copy link

    /// <summary>
    /// DO NOT use this directly, use the extension method on the exception object to call 'Log()'.
    /// </summary>
    /// <param name="message">
    /// The message.
    /// </param>
    /// <param name="source">
    /// The source.
    /// </param>
    /// <param name="stackTrace">
    /// The stack Trace.
    /// </param>
    /// <param name="targetSite">
    /// The target Site.
    /// </param>
    [Event(1, Level = EventLevel.Error, Channel = EventChannel.Operational, Message = "ExceptionCaught: {0}, Source: {1}")]
    public void ExceptionCaught(string message, string source, string stackTrace, string targetSite)
    {
        Trace.TraceError(message);

        if (this.IsEnabled())
        {
            this.WriteEvent(1, message, source, stackTrace, targetSite);
        }
    }

@FutureGUIs
Copy link

It certainly doesn't seem like a certain type of file, just anything with 3 slashes

@sharwell
Copy link
Member

Found the problem

You have documentation comments disabled for your project, because you did not check the box to create an XML documentation file during the build. You need to enable this check box if you want to use documentation-style comments in a project. I enable this using a custom <PropertyGroup> element in my .csproj files.

📝 This property group must appear after the property group(s) that define OutputPath and AssemblyName.

<PropertyGroup>
  <!--
    Make sure any documentation comments which are included in code get checked for syntax during the build, but do
    not report warnings for missing comments.

    CS1573: Parameter 'parameter' has no matching param tag in the XML comment for 'parameter' (but other parameters do)
    CS1591: Missing XML comment for publicly visible type or member 'Type_or_Member'
  -->
  <DocumentationFile>$(OutputPath)$(AssemblyName).xml</DocumentationFile>
  <NoWarn>$(NoWarn),1573,1591</NoWarn>
</PropertyGroup>

Related issues:

@FutureGUIs
Copy link

That didn't help. Plus, did that change as that wasn't how the rule was working before?

@sharwell
Copy link
Member

That didn't help.

Try doing a full rebuild and/or restarting Visual Studio after changing the setting. I've gone back and forth a few times now, and I can reproduce exactly what was indicated above by enabling and disabling this setting.

Plus, did that change as that wasn't how the rule was working before?

Not that I can see. It's possible Roslyn changed the way documentation comments appear in the parse tree in a way that affects this analyzer. I'm not entirely sure yet.

@pdelvo
Copy link
Member

pdelvo commented Jul 21, 2015

I wasnt able to repro this in VS 2015 rc (have not updated my laptop yet). So this might have changed in roslyn rtm.

@jjmartin
Copy link
Author

Intellisense isn't highlighting the issue, but that file corresponds to the selected row in my screenshot.

gonna try out the documentation comments property you mentioned above - but it seems like an odd requirement - the IDE clearly knows its a comment - its not green, its grey.

@jjmartin
Copy link
Author

if it helps (sorry i haven't really looked into the code at all of the analyzers) when i first started up VS, the 1005 warnings are all legit.

After running Code Analysis on Solution, and it completes, the warnings start to stack up, as i double click a line and it brings up that file, that file's warnings disappear off the list, (i think i was wrong - the file does't correspond to the list item - but i double clicked one of the warnings to get to that file).

its like it detects it when the file isn't open but then realizes there is no problem once i open the file.

the warnings also show up on the output from the build analysis
image

@jjmartin
Copy link
Author

I am seeing the behavior of the Build->Output->Xml Documentation File checkbox being checked removes the non-legit SA1005 warnings. (After changing, saving the project and re-running analysis on the project (with my Error List filtered for just the current project) (I also get a bunch of new warnings like CS1573)

But that doesn't seem like desired behavior that we want. It shouldn't matter if i am outputting XML documentation or not.

@sharwell
Copy link
Member

I also get a bunch of new warnings like CS1573

If this is the first time you are validating XML comments in your solution, I highly recommend using the XML PropertyGroup I posted above. You will need to manually adding it to your project file. You can see it disables 1573 and 1591 along with an explanation of why they are being disabled.

But that doesn't seem like desired behavior that we want. It shouldn't matter if i am outputting XML documentation or not.

It shouldn't, but it does. See the Roslyn bug I filed above for my request that it not differentiate behavior on this setting.

@jjmartin
Copy link
Author

ok - so enabling the XML docs is more of a work-around than a solution at this point. the other option is that we don't care that much about a space after // so i can just ignore SA1005 (for now anyway). Thanks for the tip on the nowarn, i kinda knew what was going on but looking at the Roslyn bug makes them more insidious since the usings are getting removed i suppose.

@pdelvo
Copy link
Member

pdelvo commented Jul 21, 2015

Roslyn has a parse mode that treats documentation comments as regular comments. We should support that. If @sharwell is okay with that im going to fix that.

@sharwell
Copy link
Member

@pdelvo Sure, just be sure to clearly state the semantics of your fix. 😄

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

4 participants