Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions lib/cc/engine/analyzers/reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def initialize(engine_config, language_strategy, io)
@engine_config = engine_config
@language_strategy = language_strategy
@io = io
@reports = Set.new
end

def run
Expand All @@ -35,7 +36,12 @@ def process_files

def report
flay.report(StringIO.new).each do |issue|
io.puts "#{new_violation(issue).to_json}\0"
violation = new_violation(issue)

unless reports.include?(violation.report_name)
reports.add(violation.report_name)
io.puts "#{violation.format.to_json}\0"
end
end
end

Expand All @@ -46,6 +52,8 @@ def process_sexp(sexp)

private

attr_reader :reports

def flay
@flay ||= Flay.new(flay_options)
end
Expand All @@ -58,7 +66,7 @@ def mass_threshold

def new_violation(issue)
hashes = flay.hashes[issue.structural_hash]
Violation.new(language_strategy.base_points, issue, hashes).format
Violation.new(language_strategy.base_points, issue, hashes)
end

def flay_options
Expand Down
4 changes: 4 additions & 0 deletions lib/cc/engine/analyzers/violation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ def format
}
end

def report_name
"#{current_sexp.file}-#{current_sexp.line}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this include end line as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about that myself, and I'm not sure honestly. I don't think there would be any (or very many at the least) instances where the same start line has multiple end-lines.

I'd love to hear what others think about that though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool - You know more about this so I defer to you on that!

end

private

attr_reader :base_points, :hashes
Expand Down
10 changes: 10 additions & 0 deletions spec/cc/engine/analyzers/javascript/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@
expect(run_engine(engine_conf)).to be_empty
end

it "does not report the same line for multiple issues" do
create_source_file("dup.jsx", <<-EOJSX)
<a className='button button-primary full' href='#' onClick={this.onSubmit.bind(this)}>Login</a>
EOJSX

result = run_engine(engine_conf).strip
issues = result.split("\0")
expect(issues.length).to eq 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@BlakeWilliams can you explain why this is 1? Is it because we set the mass to 1 for the tests? I wouldn't expect a line to be reported unless there was another instance of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it's because we pass in an extremely low mass threshold so this will always be higher.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thanks!

end

def create_source_file(path, content)
File.write(File.join(@code, path), content)
end
Expand Down