[Suggestion] Ignore DS137138 in xml documentation and xml namespaces about devskim HOT 22 CLOSED

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

from devskim.

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

from devskim.

Affects WIX:

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

from devskim.

@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.

from devskim.

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

from devskim.

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

from devskim.

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...).

from devskim.

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.

from devskim.

@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.

from devskim.

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>

from devskim.

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

from devskim.

@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

from devskim.

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

from devskim.

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" ]

from devskim.

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

Otherwise this really meaningful extension is becomming completely useless.

from devskim.

@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.

from devskim.

This is still an issue, i propose reopening it

from devskim.

@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

from devskim.

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

from devskim.

@PavelBansky Any updates/plans for VS2017 Extension yet?

from devskim.

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.

from devskim.

I'm closing this issue for now.

from devskim.