Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Raise default remediation, categorize checks #19

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 7, 2015
Merged

Raise default remediation, categorize checks #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9da9360
Raise default remediation, categorize checks
pbrisbin Dec 7, 2015
e17113a
Use a constant
pbrisbin Dec 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/cc/engine/csslint.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
module CC
module Engine
class CSSlint
autoload :CheckDetails, "cc/engine/csslint/check_details"

def initialize(directory: , io: , engine_config: )
@directory = directory
@engine_config = engine_config
Expand All @@ -18,13 +20,15 @@ def run
next unless node.name == "error"

lint = node.attributes
check_name = lint["source"].value
check_details = CheckDetails.fetch(check_name)

issue = {
type: "issue",
check_name: lint["source"].value,
check_name: check_name,
description: lint["message"].value,
categories: ["Style"],
remediation_points: 500,
categories: check_details.categories,
remediation_points: check_details.remediation_points,
location: {
path: path,
positions: {
Expand Down
53 changes: 53 additions & 0 deletions lib/cc/engine/csslint/check_details.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
module CC
module Engine
class CSSlint
class CheckDetails
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the major function of this class at present is to get the category for an issue.

What do you think about having a category class instead, and have it initialize with a check?

Then, in the CSSLint or an Issue class, we can have

def categories
  Category.new(check_name)
end

def remediation_points
   DEFAULT_REMEDIATION_POINTS
end

ALL_RULES = {
# https://github.com/CSSLint/csslint/wiki/Rules
"net.csslint.Adjoiningclasses" => { categories: "Compatability" },
"net.csslint.Boxmodel" => { categories: "Bug Risk" },
"net.csslint.Boxsizing" => { categories: "Compatability" },
"net.csslint.Bulletprooffontface" => { categories: "Compatability" },
"net.csslint.Compatiblevendorprefixes" => { categories: "Compatability" },
"net.csslint.Displaypropertygrouping" => { categories: "Bug Risk" },
"net.csslint.Duplicatebackgroundimages" => { categories: "Bug Risk" },
"net.csslint.Duplicateproperties" => { categories: "Bug Risk" },
"net.csslint.Emptyrules" => { categories: "Bug Risk" },
"net.csslint.Fallbackcolors" => { categories: "Compatability" },
"net.csslint.Fontfaces" => { categories: "Bug Risk" },
"net.csslint.Gradients" => { categories: "Compatability" },
"net.csslint.Import" => { categories: "Bug Risk" },
"net.csslint.Knownproperties" => { categories: "Bug Risk" },
"net.csslint.Overqualifiedelements" => { categories: "Bug Risk" },
"net.csslint.Regexselectors" => { categories: "Bug Risk" },
"net.csslint.Shorthand" => { categories: "Bug Risk" },
"net.csslint.Starpropertyhack" => { categories: "Compatability" },
"net.csslint.Textindent" => { categories: "Compatability" },
"net.csslint.Underscorepropertyhack" => { categories: "Compatability" },
"net.csslint.Uniqueheadings" => { categories: "Duplication" },
"net.csslint.Universalselector" => { categories: "Bug Risk" },
"net.csslint.Unqualifiedattributes" => { categories: "Bug Risk" },
"net.csslint.Vendorprefix" => { categories: "Compatability" },
"net.csslint.Zerounits" => { categories: "Bug Risk" },
}.freeze

DEFAULT_CATEGORY = "Style".freeze
DEFAULT_REMEDIATION_POINTS = 50_000.freeze

attr_reader :categories, :remediation_points

def self.fetch(check_name)
new(ALL_RULES.fetch(check_name, {}))
end

def initialize(
categories: DEFAULT_CATEGORY,
remediation_points: DEFAULT_REMEDIATION_POINTS
)
@categories = Array(categories)
@remediation_points = remediation_points
end
end
end
end
end
21 changes: 21 additions & 0 deletions spec/cc/engine/csslint/check_details_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require "spec_helper"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! glad to have some tests.


class CC::Engine::CSSlint
describe CheckDetails do
describe ".fetch" do
it "returns details for customized checks" do
details = CheckDetails.fetch("net.csslint.Import")

expect(details.categories).to eq ["Bug Risk"]
expect(details.remediation_points).to eq 50_000
end

it "returns defaults for unknown checks" do
details = CheckDetails.fetch("made-up")

expect(details.categories).to eq ["Style"]
expect(details.remediation_points).to eq 50_000
end
end
end
end
3 changes: 1 addition & 2 deletions spec/cc/engine/csslint_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'cc/engine/csslint'
require 'tmpdir'
require "spec_helper"

module CC
module Engine
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
require "cc/engine/csslint"
require "tmpdir"