[Suggestion] Ignore DS137138 in xml documentation and xml namespaces

[ChristophvanderFecht]

So far I am happy with the plugin, but most of the time I am ignoring errors for web pages that are mentioned in the xml docu or xml namespace (which btw. kills visual studio 2015). It would be nice if there would be an option to ignore links in the xml docu like:

/// <para tool="javadoc-to-mdoc"> /// <format type="text/html"> /// <a href="http://developer.android.com/reference/android/app/Activity.html#onStop()" target="_blank">[Android Documentation]</a> /// </format> /// </para>

Bg

[joshbw]

An improvement we should broadly make is the ability to say "this rule doesn't apply to this code type". Right now we support "applies_to" which says what languages it applies to, and if left empty it applies to everything. The flaw in that is that if we want to allow everything BUT one or two code types, then we need to call out the dozen languages it should apply to, rather than the two it shouldn't. It isn't a hard change, so we should probably add that to the schema @PavelBansky & @scovetta

[PavelBansky]

Do we have some existing example where this will be needed, or is it just an idea?

[scovetta]

VS projects drop a build.props file, which start out with something like:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>

DevSkim triggers on that second like (http://schemas...).

[Lexcess]

I hit on this as well. The rule triggers on .csproj files that don't use the new SDK attribute (<Project Sdk="Microsoft.NET.Sdk">). I suspect, although haven't checked, this will be the case for other languages such as C++, F# and VB. For example:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
...

As well as csproj, if you want to add MSBuild files using the props (as @scovetta mentions) or targets convention you also get a hit with this rule. For example:

A valid props file that triggers the rule

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <PropertyGroup>
      <MyProperty>Hello</MyProperty>
    </PropertyGroup>
</Project>

A valid targets file that triggers the rule

<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Target Name="Example" BeforeTargets="CoreCompile" >
         <Message Importance="High" Text="Hello World" />
  </Target>
</Project>

Obviously these all come from the same root; the xml namespace "http://schemas.microsoft.com/developer/msbuild/2003" is in violation, but there is little a developer can do given these are expected and mostly auto-generated.

[scovetta]

@joshbw Does the VSCode engine support negative lookbehinds in the regex? If so, we might be able to handle this specific instance with (?<!xmlns=")http:. Otherwise, we'll have to wait until conditions are in.

[the-ress]

xmlns:shorthand="http://..." also should be ignored:

<UserControl xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             ...>
...
</UserControl>

[Lexcess]

Another one for the pile: NamespaceAttribute.

Running on a codebase with a generated SOAP proxy results in hundreds of violations.

Seeing stuff like:

[System.CodeDom.Compiler.GeneratedCodeAttribute("wsdl", "2.0.50727.3038")]
...
[System.Web.Services.WebServiceBindingAttribute(Name="MyService", Namespace="http://www.example.com/MyService")]

or

[return: System.Xml.Serialization.XmlElementAttribute("MyObject", Namespace="http://www.example.com/MyService")]

This example is from code generated by an older tool but it is still not something a developer can really change (especially as a client) and throws up a ton of noise when trying to review code

[scovetta]

Thanks @Lexcess for the samples -- we'll make sure we cover these!

[lennybacon]

Affects WIX:

<Wix
    xmlns="http://schemas.microsoft.com/wix/2006/wi"
    xmlns:util="http://schemas.microsoft.com/wix/UtilExtension">

[joshbw]

@scovetta JavaScript doesn't have LookBehind (though it has LookAhead), so it's non-trivial to support. I think our "BeforeFinding" support function would be easier to implement

[Xceno]

I have a similar issue, but with a different rule (DS173237).
It's a XML file with some GUIDs in it which triggers this.

[joshbw]

We'll take a look at these rules. We are working on an update to the rules engine to allow us to pull in more context in our rules (like checking if a pattern occurs just before xmlns and NOT triggering it). In the interim, you can disable specific rules or analysis in a particular file in the DevSkim settings

in the VS Code version of the plugin the setting would look like

"devskim.ignoreRulesList": ["DS173237","DS137138"]

or

"devskim.ignoreFilesList": [ "out/*", "bin/*", "node_modules/*", ".vscode/*", "yarn.lock", "logs/*", "*.log", "*.xml" ]

[lennybacon]

@joshbw What would be the setting in VS and where to set it.

Otherwise this really meaningful extension is becomming completely useless.

[PavelBansky]

@lennybacon , what would you prefer that feature look a like?

I'm implementing that functionality in VS and would be happy to get a feedback on potential approaches:

1. Settings page where you select rule ID and flag it as disabled. Same thing for the path.

2. Settings page with free form text area where you can put comma delimited list of IDs. So, you can easily copy/paste lists between VS settings and VS Code settings.

Both options can come with import/export feature from/into VS Code compatible JSON settings.

[lennybacon]

@PavelBansky, I think option 1 makes more sense, as the entry level for this extension should be as low as possible to make its reach as broad as possible.

[RonaldPhilipsen]

This is still an issue, i propose reopening it

[joshbw]

@Ronald245 which client are you using (VS, VS Code, Sublime)? We added the ability to have additional conditions fire to hone in on an issue, so we now invalidate a finding if it is the xmlns, but it is possible we forgot to update one of the clients with the altered rule

[RonaldPhilipsen]

@joshbw Visual studio 2017, it's worth noting however that i havent seen the extension update in a while,

[PavelBansky]

Sorry, all the effort went to Command Line Tool and making rules better. The VS 2017 extension will be updated next.

[lennybacon]

@PavelBansky Any updates/plans for VS2017 Extension yet?

[PavelBansky]

The latest version (0.3.1) is now available in the VS gallery.
It has the fix for "xmlns="http://schemas.microso" issues.

The arbitrary enabling and disabling of individual rules is not, yet implemented in this version.

[PavelBansky]

I'm closing this issue for now.