Skip to content

Rewrite DevSkim to use the Application Inspector Engine - #390

Merged
Giulia Stocco (gfs) merged 33 commits into
mainfrom
gfs/AiRewrite
Aug 12, 2022
Merged

Giulia Stocco (gfs) merged 33 commits into
mainfrom
gfs/AiRewrite

Conversation

@gfs

@gfs Giulia Stocco (gfs) commented Jul 27, 2022 •

Copy link
Copy Markdown
Contributor

This is intended to complete #367.

Fixes #389
Fixes #148
Fixes #364

Also planned for this refactor, but not present in this PR:
#384
#385
#383
#381
#343
#303
#289
#217

Refactor DevSkim to use AI engine. I'm refactoring it in a separate project, which will then be tested against the original devskim using the same rules to validate behavior, then the "DevSkim.AI" project will be renamed back to "Microsoft.DevSkim" and the old version removed.
Trimmed down to the Analyze and Verify commands by leveraging the refactored AI based engine.

Pack command is no longer needed. Multiple rules files are simply embedded.
Catalogue command not implemented for now. I'm not clear on how useful it is.
Test command not implemented for now. Also not clear how much use it sees.
Adds initial test to validate the default rules.
Cleans up the gitignore in the dotnet dir.
CLI Analyze Now Runs and appears to be working.
Remainder of Enums which used dashes have been renamed.
Now load DevSkim's languages and comments rather than using App inspectors.
Confirmed via some manual testing that new implementation does not miss any matches the old one found. And in fact it finds some matches that were not appearing with old engine.
public void DenamespacedRule()
{
var content = @"<?xml version=""1.0"" encoding=""UTF-8""?>
<project xmlns=""http://maven.apache.org/POM/4.0.0"" xmlns:xsi=""http://www.w3.org/2001/XMLSchema-instance"" xsi:schemaLocation=""http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"">

Check failure

Code scanning / devskim

Insecure URL

Insecure URL
To allow refactored CLI build to work properly. Will be restored when rewritten extension is added back.
@gfs
Giulia Stocco (gfs) marked this pull request as ready for review August 11, 2022 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[False Positive] - Localhost almost always use the non-TLS HTTP URI scheme Add option to skip including excerpt Ignore files in .gitignore

2 participants