Skip to content

Commit 1091110

Browse files
zenspiderAle Paredes
authored and
Ale Paredes
committed
Move flay to do pre-filtering, not post-filtering. (#313)
* Bump flay and sexp_processor for new pre-filtering option. This also removes json, as that ships with every ruby these days. I had trouble building it, which seems unnecessary to even do. I *think* I manually updated this lockfile properly. I don't have /home/app/codeclimate-parser-client so I don't know how you're supposed to use bundler properly. I basically updated and then carved out some of the diffs until everything ran clean. * [WIP] roll back #206 This has been merged upstream. * [WIP] remove `delete_comments!` in favor of using `filters` * Improved filtering of "false positives" for various languages. + Fixed filtering for go. It was too narrow. + Improved engine_conf in most specs to allow for variable mass thresholds. + Ensured that filter specs have actual bodies so we're not wiping everything. * Bump flay, ruby_parser, and sexp_processor.
1 parent fd170dc commit 1091110

File tree

9 files changed

+29
-50
lines changed

9 files changed

+29
-50
lines changed

Diff for: Gemfile

+2-3
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
source "https://rubygems.org"
33

44
gem "concurrent-ruby", "~> 1.0.0"
5-
gem "flay", "~> 2.10"
6-
gem "json"
7-
gem "sexp_processor", "~> 4.10"
5+
gem "flay", "~> 2.12"
6+
gem "sexp_processor", "~> 4.11"
87

98
gem "codeclimate-parser-client",
109
path: "/home/app/codeclimate-parser-client"

Diff for: Gemfile.lock

+6-8
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,13 @@ GEM
1414
diff-lcs (1.2.5)
1515
erubis (2.7.0)
1616
excon (0.57.1)
17-
flay (2.10.0)
17+
flay (2.12.0)
1818
erubis (~> 2.7.0)
1919
path_expander (~> 1.0)
2020
ruby_parser (~> 3.0)
2121
sexp_processor (~> 4.0)
22-
json (1.8.3)
2322
method_source (0.8.2)
24-
path_expander (1.0.2)
23+
path_expander (1.0.3)
2524
pry (0.10.3)
2625
coderay (~> 1.1.0)
2726
method_source (~> 0.8.1)
@@ -40,9 +39,9 @@ GEM
4039
diff-lcs (>= 1.2.0, < 2.0)
4140
rspec-support (~> 3.3.0)
4241
rspec-support (3.3.0)
43-
ruby_parser (3.10.0)
42+
ruby_parser (3.11.0)
4443
sexp_processor (~> 4.9)
45-
sexp_processor (4.10.0)
44+
sexp_processor (4.11.0)
4645
slop (3.6.0)
4746

4847
PLATFORMS
@@ -51,12 +50,11 @@ PLATFORMS
5150
DEPENDENCIES
5251
codeclimate-parser-client!
5352
concurrent-ruby (~> 1.0.0)
54-
flay (~> 2.10)
55-
json
53+
flay (~> 2.12)
5654
pry
5755
rake
5856
rspec
59-
sexp_processor (~> 4.10)
57+
sexp_processor (~> 4.11)
6058

6159
BUNDLED WITH
6260
1.10.6

Diff for: lib/cc/engine/analyzers/go/main.rb

+2-7
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,13 @@ class Main < CC::Engine::Analyzers::Base
1414
PATTERNS = ["**/*.go"].freeze
1515
DEFAULT_MASS_THRESHOLD = 100
1616
DEFAULT_FILTERS = [
17-
"(ImportSpec ___)",
17+
"(GenDecl _ (specs (ImportSpec ___)) _)",
18+
"(comments ___)",
1819
].freeze
1920
POINTS_PER_OVERAGE = 10_000
2021
REQUEST_PATH = "/go"
21-
COMMENT_MATCHER = Sexp::Matcher.parse("(_ (comments ___) ___)")
2222

2323
def transform_sexp(sexp)
24-
delete_comments!(sexp)
2524
sexp.delete_if { |node| node[0] == :name }
2625
end
2726

@@ -38,10 +37,6 @@ def process_file(file)
3837
def default_filters
3938
DEFAULT_FILTERS.map { |filter| Sexp::Matcher.parse filter }
4039
end
41-
42-
def delete_comments!(sexp)
43-
sexp.search_each(COMMENT_MATCHER) { |node| node.delete_at(1) }
44-
end
4540
end
4641
end
4742
end

Diff for: lib/cc/engine/analyzers/php/main.rb

+1-9
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,10 @@ class Main < CC::Engine::Analyzers::Base
1414
DEFAULT_MASS_THRESHOLD = 75
1515
DEFAULT_FILTERS = [
1616
"(Stmt_Use ___)",
17+
"(comments ___)",
1718
].freeze
1819
POINTS_PER_OVERAGE = 35_000
1920
REQUEST_PATH = "/php"
20-
COMMENT_MATCHER = Sexp::Matcher.parse("(_ (comments ___) ___)")
21-
22-
def transform_sexp(sexp)
23-
delete_comments!(sexp)
24-
end
2521

2622
def use_sexp_lines?
2723
false
@@ -36,10 +32,6 @@ def process_file(file)
3632
def default_filters
3733
DEFAULT_FILTERS.map { |filter| Sexp::Matcher.parse filter }
3834
end
39-
40-
def delete_comments!(sexp)
41-
sexp.search_each(COMMENT_MATCHER) { |node| node.delete_at(1) }
42-
end
4335
end
4436
end
4537
end

Diff for: lib/ccflay.rb

-11
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
require "flay"
44
require "concurrent"
55
require "digest"
6-
require "zlib"
76

87
##
98
# A thread-safe and stable hash subclass of Flay.
@@ -21,16 +20,6 @@ def initialize(option = nil)
2120
end
2221
end
2322

24-
# Overwrite `NODE_NAMES` from Flay to assign all values on-demand instead of
25-
# using a predefined registry.
26-
Sexp::NODE_NAMES.delete_if { true }
27-
28-
Sexp::NODE_NAMES.default_proc = lambda do |hash, key|
29-
# Use CRC checksums so hash values are order-independent (i.e. consistent
30-
# between runs).
31-
hash[key] = Zlib.crc32(key.to_s)
32-
end
33-
3423
class Sexp
3524
attr_writer :mass
3625

Diff for: spec/cc/engine/analyzers/go/main_spec.rb

+3-3
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ module CC::Engine::Analyzers
123123
}
124124
EOGO
125125

126-
issues = run_engine(engine_conf).strip.split("\0")
126+
issues = run_engine(engine_conf 25).strip.split("\0")
127127
expect(issues).to be_empty
128128
end
129129

@@ -218,7 +218,7 @@ module CC::Engine::Analyzers
218218
expect(run_engine(engine_conf)).to be_empty
219219
end
220220

221-
def engine_conf
221+
def engine_conf mass = 10
222222
CC::Engine::Analyzers::EngineConfig.new({
223223
'config' => {
224224
'checks' => {
@@ -231,7 +231,7 @@ def engine_conf
231231
},
232232
'languages' => {
233233
'go' => {
234-
'mass_threshold' => 10,
234+
'mass_threshold' => mass,
235235
},
236236
},
237237
},

Diff for: spec/cc/engine/analyzers/javascript/main_spec.rb

+5-3
Original file line numberDiff line numberDiff line change
@@ -159,16 +159,18 @@
159159
b = require('bar'),
160160
c = require('baz'),
161161
d = require('bam');
162+
a + b + c + d;
162163
EOJS
163164

164165
create_source_file("bar.js", <<~EOJS)
165166
const a = require('foo'),
166167
b = require('bar'),
167168
c = require('baz'),
168169
d = require('bam');
170+
print(a);
169171
EOJS
170172

171-
issues = run_engine(engine_conf).strip.split("\0")
173+
issues = run_engine(engine_conf 3).strip.split("\0")
172174
expect(issues).to be_empty
173175
end
174176

@@ -201,7 +203,7 @@
201203
})
202204
end
203205

204-
def engine_conf
206+
def engine_conf mass = 1
205207
CC::Engine::Analyzers::EngineConfig.new({
206208
'config' => {
207209
'checks' => {
@@ -214,7 +216,7 @@ def engine_conf
214216
},
215217
'languages' => {
216218
'javascript' => {
217-
'mass_threshold' => 1,
219+
'mass_threshold' => mass,
218220
},
219221
},
220222
},

Diff for: spec/cc/engine/analyzers/php/main_spec.rb

+5-3
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
use KeepClear\\Traits\\Controllers\\ApiFilter;
178178
use KeepClear\\Traits\\Controllers\\ApiParseBody;
179179
use KeepClear\\Traits\\Controllers\\ApiException;
180+
a / b;
180181
EOPHP
181182

182183
create_source_file("bar.php", <<~EOPHP)
@@ -190,9 +191,10 @@
190191
use KeepClear\\Traits\\Controllers\\ApiFilter;
191192
use KeepClear\\Traits\\Controllers\\ApiParseBody;
192193
use KeepClear\\Traits\\Controllers\\ApiException;
194+
a + b;
193195
EOPHP
194196

195-
issues = run_engine(engine_conf).strip.split("\0")
197+
issues = run_engine(engine_conf 6).strip.split("\0")
196198
expect(issues).to be_empty
197199
end
198200

@@ -275,12 +277,12 @@
275277
end
276278
end
277279

278-
def engine_conf
280+
def engine_conf mass = 5
279281
CC::Engine::Analyzers::EngineConfig.new({
280282
'config' => {
281283
'languages' => {
282284
'php' => {
283-
'mass_threshold' => 5,
285+
'mass_threshold' => mass,
284286
},
285287
},
286288
},

Diff for: spec/cc/engine/analyzers/typescript/main_spec.rb

+5-3
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,18 @@
140140
b = require('bar'),
141141
c = require('baz'),
142142
d = require('bam');
143+
print(a);
143144
EOTS
144145

145146
create_source_file("bar.ts", <<~EOTS)
146147
const a = require('foo'),
147148
b = require('bar'),
148149
c = require('baz'),
149150
d = require('bam');
151+
c * d;
150152
EOTS
151153

152-
issues = run_engine(engine_conf).strip.split("\0")
154+
issues = run_engine(engine_conf 3).strip.split("\0")
153155
expect(issues).to be_empty
154156
end
155157

@@ -209,7 +211,7 @@
209211
expect(json["fingerprint"]).to eq("d8f0315c3c4e9ba81003a7ec6c823fb0")
210212
end
211213

212-
def engine_conf
214+
def engine_conf mass = 1
213215
CC::Engine::Analyzers::EngineConfig.new({
214216
'config' => {
215217
'checks' => {
@@ -222,7 +224,7 @@ def engine_conf
222224
},
223225
'languages' => {
224226
'typescript' => {
225-
'mass_threshold' => 1,
227+
'mass_threshold' => mass,
226228
},
227229
},
228230
},

0 commit comments

Comments
 (0)