Skip to content

Commit 8ab0feb

Browse files
ariskataokaariskataoka
authored
Add logic to skip files with more than 15000 lines (#186)
* Add logic to skip files with more than 15000 lines History: Add validation Add file validation Introduce file validation before phpcs scan Add submit message about skipped files to GH Update validation Refactor to apply validation as an array Remove unnecessary function Fix unit test Add require for file validation Modify test to assert equals instead of same Currently, it's been trying to assert two different objects, when the object only need to have the same attribute values Fix small issue Stops forcing for tests Fix array structure for svg scan Add skip-file logic into a separated file Add validation/skip files logic in the lint scan Add verification to check if the file is skipped Add back code block removed by mistake during rebase Add test for skipped files due to limit of lines exceeded Return 250 when skipped files due to files exceeded issue Add tests for skip files funtions Move message functions for skipped files to skip-file Adds test Address bot comments Add filename in the output for file validation Add comment to explain why bot returns non-zero when find skipped files Add code removed by rebase Add strict types/return types for the new files. Sanitizes and verify if line count command output is numeric Fix test mock Fix verification if files has been already scanned in php-cs process Add cache layer to file validation Replace assertequals with assertsame Refactor cache logic to use cache key as array and value as string * Add option skip-large-files * Apply automattic indent in the modified test files * Add option skip-large-files-limit Default is set to 15000 * Add helper for new parameters skip-large-file and limit
1 parent 1ffdc92 commit 8ab0feb

17 files changed

+1584
-335
lines changed

‎defines.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,23 @@
134134
* Define for vipgoci_git_diffs_fetch()
135135
*/
136136

137-
define( 'VIPGOCI_GIT_DIFF_CALC_CHANGES', array ('+' => 'additions', '-' => 'deletions') );
137+
define( 'VIPGOCI_GIT_DIFF_CALC_CHANGES', array ('+' => 'additions', '-' => 'deletions') );
138138
define( 'VIPGOCI_GIT_DIFF_DATA_SOURCE_GIT_REPO', 'local-git-repo' );
139139
define( 'VIPGOCI_GIT_DIFF_DATA_SOURCE_GITHUB_API', 'github-api' );
140140

141+
142+
143+
/**
144+
* Define file number of lines limit
145+
*/
146+
define( 'VIPGOCI_SKIPPED_FILES', 'skipped-files' );
147+
148+
define( 'VIPGOCI_VALIDATION_MAXIMUM_LINES_LIMIT', 15000 );
149+
define( 'VIPGOCI_VALIDATION_MAXIMUM_LINES', 'max-lines' );
150+
define(
151+
'VIPGOCI_VALIDATION',
152+
[
153+
VIPGOCI_VALIDATION_MAXIMUM_LINES
154+
=> 'Maximum number of lines exceeded (%d)'
155+
]
156+
);

‎file-validation.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
<?php
2+
declare( strict_types=1 );
3+
4+
/**
5+
* Validation applied to verify wether the bot should scan or skip the file specified
6+
*/
7+
8+
/**
9+
* @param string $temp_file_name
10+
* @param string $file_name
11+
* @param string $commit_id
12+
* @param int $max_lines
13+
*
14+
* Validates if the file is valid to be scanned by vip-go-ci
15+
*
16+
* @return array
17+
* [
18+
* 'issues' =>
19+
* [
20+
* 'max-lines' => [$file_name],
21+
* ],
22+
* 'total' => 1
23+
* ]
24+
* In a oop we could simply inject a service for caching
25+
* Vars and set values in the constructor
26+
*/
27+
function vipgoci_validate( string $temp_file_name, string $file_name, string $commit_id, int $max_lines ): array {
28+
$validation_result = array( 'total' => 0 );
29+
30+
if ( false === vipgoci_is_number_of_lines_valid( $temp_file_name, $file_name, $commit_id, $max_lines ) ) {
31+
$validation_result['issues'][ VIPGOCI_VALIDATION_MAXIMUM_LINES ] = [ $file_name ];
32+
$validation_result['total'] = count( $validation_result['issues'] );
33+
}
34+
35+
return $validation_result;
36+
}
37+
38+
/**
39+
* @param string $temp_file_name
40+
* @param string $file_name
41+
* @param string $commit_id
42+
* @param int $max_lines
43+
*
44+
* @return bool
45+
*/
46+
function vipgoci_is_number_of_lines_valid( string $temp_file_name, string $file_name, string $commit_id, int $max_lines ): bool {
47+
/**
48+
* Verifies if number of lines validation are in the cache
49+
* If so, returns the value
50+
*/
51+
52+
$cache_key = vipgoci_cache_get_is_number_of_lines_valid_key( $file_name, $commit_id );
53+
$is_number_of_lines_valid = vipgoci_cache_get_is_number_of_lines_valid( $cache_key );
54+
if ( ! is_null( $is_number_of_lines_valid ) ) {
55+
return $is_number_of_lines_valid;
56+
}
57+
58+
/**
59+
* Calculates the file number of lines
60+
* if it is > than the default limit (defined at defined)
61+
* the bot won't scan it
62+
*/
63+
$cmd = sprintf( 'wc -l %s | awk \'{print $1;}\' 2>&1', escapeshellcmd( $temp_file_name ) );
64+
vipgoci_log( 'Validating number of lines', array( 'file_name' => $file_name ) );
65+
66+
$output = vipgoci_sanitize_string(
67+
vipgoci_runtime_measure_shell_exec( $cmd, 'file_validation' )
68+
);
69+
70+
vipgoci_log(
71+
'Validation number of lines ',
72+
array( 'file_name' => $file_name, 'cmd' => $cmd, 'output' => $output )
73+
);
74+
75+
$is_number_of_lines_valid = vipgoci_verify_number_of_lines_output( $output, $max_lines );
76+
77+
vipgoci_cache_set_is_number_of_lines_valid( $cache_key, $is_number_of_lines_valid );
78+
79+
return $is_number_of_lines_valid;
80+
}
81+
82+
/**
83+
* @param array $cache_key
84+
* @param bool $is_number_of_lines_valid
85+
*
86+
* Sets cache for converted is_number_of_lines_valid
87+
*/
88+
function vipgoci_cache_set_is_number_of_lines_valid( array $cache_key, bool $is_number_of_lines_valid ): void {
89+
vipgoci_cache(
90+
$cache_key,
91+
$is_number_of_lines_valid === true ? 'true' : 'false'
92+
);
93+
}
94+
95+
/**
96+
* @param string $output
97+
* @param int $max_lines
98+
*
99+
* @return bool
100+
*/
101+
function vipgoci_verify_number_of_lines_output( string $output, int $max_lines ): bool {
102+
return is_numeric( $output ) && $output < $max_lines;
103+
}
104+
105+
/**
106+
* @param array $cache_key
107+
*
108+
* Verifies if there's cached validation result available
109+
* returns it if it does, else returns null
110+
*
111+
* @return bool|null
112+
*/
113+
function vipgoci_cache_get_is_number_of_lines_valid( array $cache_key ): ?bool {
114+
$cached_value = vipgoci_cache( $cache_key );
115+
116+
return false !== $cached_value ? 'true' === $cached_value : null;
117+
}
118+
119+
/**
120+
* @param string $file_name
121+
* @param string $commit_id
122+
* Builds the cache key for the number of lines validation
123+
*
124+
* @return array
125+
*/
126+
function vipgoci_cache_get_is_number_of_lines_valid_key( string $file_name, string $commit_id ): array {
127+
return array( __FUNCTION__, $file_name, $commit_id );
128+
}

‎github-api.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,7 +2593,6 @@ function vipgoci_github_pr_review_submit(
25932593
}
25942594
}
25952595

2596-
25972596
/*
25982597
* If there are no issues to report to GitHub,
25992598
* do not continue processing the Pull-Request.
@@ -2602,7 +2601,8 @@ function vipgoci_github_pr_review_submit(
26022601
if (
26032602
( false === $github_errors ) &&
26042603
( false === $github_warnings ) &&
2605-
( false === $github_info )
2604+
( false === $github_info ) &&
2605+
empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ] )
26062606
) {
26072607
continue;
26082608
}
@@ -2718,6 +2718,16 @@ function vipgoci_github_pr_review_submit(
27182718
}
27192719
}
27202720

2721+
/**
2722+
* Format skipped files message if it validation has issues
2723+
*/
2724+
if ( ! empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ] ) ) {
2725+
$github_postfields[ 'body' ] .= vipgoci_get_skipped_files_message(
2726+
$results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ],
2727+
$pr_number
2728+
);
2729+
}
2730+
27212731
/*
27222732
* If we have a informational-URL about
27232733
* the bot, append it along with a generic

‎lint-scan.php‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ function vipgoci_lint_parse_results(
198198
function vipgoci_lint_scan_commit(
199199
$options,
200200
&$commit_issues_submit,
201-
&$commit_issues_stats
201+
&$commit_issues_stats,
202+
array &$commit_skipped_files
202203
) {
203204
$repo_owner = $options['repo-owner'];
204205
$repo_name = $options['repo-name'];
@@ -258,6 +259,7 @@ function vipgoci_lint_scan_commit(
258259

259260
/*
260261
* Lint every PHP file existing in the commit
262+
* $commit_tree is an array of files for that commit
261263
*/
262264

263265
foreach( $commit_tree as $filename ) {
@@ -279,6 +281,31 @@ function vipgoci_lint_scan_commit(
279281
$file_contents
280282
);
281283

284+
/**
285+
* Validates the file
286+
* and if it's not valid, the scans skips it
287+
*/
288+
if ( true === $options['skip-large-files'] ) {
289+
$validation = vipgoci_validate(
290+
$temp_file_name,
291+
$filename,
292+
$commit_id,
293+
$options[ 'skip-large-files-limit' ]
294+
);
295+
if ( 0 !== $validation[ 'total' ] ) {
296+
unlink( $temp_file_name );
297+
298+
vipgoci_set_prs_implicated_skipped_files( $prs_implicated, $commit_skipped_files, $validation );
299+
vipgoci_runtime_measure( VIPGOCI_RUNTIME_STOP, 'lint_scan_single_file' );
300+
301+
continue;
302+
}
303+
}
304+
305+
/**
306+
* The lint scan will only proceed if the file is valid
307+
*
308+
*/
282309
/*
283310
* Keep statistics of what we do.
284311
*/

‎main.php‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ function vipgoci_help_print( $argv ) {
2424
"\t" . ' that expect a URL are HTTPS and not HTTP. Default is true.' . PHP_EOL .
2525
"\t" . '--skip-draft-prs=BOOL If true, skip scanning of all Pull-Requests that are in draft mode.' . PHP_EOL .
2626
"\t" . ' Default is false.' . PHP_EOL .
27+
"\t" . '--skip-large-files=true=BOOL If true, skip scanning files that have number of lines higher than the skip-large-files-limit value.' . PHP_EOL .
28+
"\t" . ' Default is true.' . PHP_EOL .
29+
"\t" . '--skip-large-files-limit=INTEGER Defines the maximum number of lines limit per file.' . PHP_EOL .
30+
"\t" . ' Default is 15000 lines.' . PHP_EOL .
2731
"\t" . '--branches-ignore=STRING,... What branches to ignore -- useful to make sure' . PHP_EOL .
2832
"\t" . ' some branches never get scanned. Separate branches' . PHP_EOL .
2933
"\t" . ' with commas.' . PHP_EOL .
@@ -224,6 +228,8 @@ function vipgoci_options_recognized() {
224228
'skip-draft-prs:',
225229
'branches-ignore:',
226230
'local-git-repo:',
231+
'skip-large-files:',
232+
'skip-large-files-limit:',
227233

228234
/*
229235
* Environmental & repo configuration
@@ -380,7 +386,15 @@ function vipgoci_exit_status( $results ) {
380386
return VIPGOCI_EXIT_CODE_ISSUES;
381387
}
382388
}
389+
}
383390

391+
if ( ! empty( $results['skipped-files'] ) ) {
392+
foreach ( $results['skipped-files'] as $pr_number ) {
393+
if( 0 < $pr_number[ 'total' ] ) {
394+
// Results contains skipped files due issues, return non-zero
395+
return VIPGOCI_EXIT_CODE_ISSUES;
396+
}
397+
}
384398
}
385399

386400
return 0;
@@ -813,10 +827,11 @@ function vipgoci_run() {
813827
range( 0, 500, 1 )
814828
);
815829

830+
vipgoci_option_integer_handle( $options, 'skip-large-files-limit', 15000 );
831+
816832
/*
817833
* Handle boolean parameters
818834
*/
819-
820835
vipgoci_option_bool_handle( $options, 'skip-draft-prs', 'false' );
821836

822837
vipgoci_option_bool_handle( $options, 'phpcs', 'true' );
@@ -829,6 +844,8 @@ function vipgoci_run() {
829844

830845
vipgoci_option_bool_handle( $options, 'lint', 'true' );
831846

847+
vipgoci_option_bool_handle( $options, 'skip-large-files', 'true' );
848+
832849
vipgoci_option_bool_handle( $options, 'lint-skip-folders-in-repo-options-file', 'false' );
833850

834851
vipgoci_option_bool_handle( $options, 'dismiss-stale-reviews', 'false' );
@@ -1654,7 +1671,7 @@ function vipgoci_run() {
16541671
VIPGOCI_GITHUB_WEB_BASE_URL . '/' .
16551672
rawurlencode( $options['repo-owner'] ) . '/' .
16561673
rawurlencode( $options['repo-name'] ) . '/' .
1657-
'commit/' .
1674+
'commit/' .
16581675
rawurlencode( $options['commit'] )
16591676
);
16601677

@@ -1671,7 +1688,7 @@ function vipgoci_run() {
16711688
vipgoci_log(
16721689
'Starting up...',
16731690
array(
1674-
'options' => vipgoci_options_sensitive_clean(
1691+
'options' => vipgoci_options_sensitive_clean(
16751692
$options
16761693
)
16771694
)
@@ -1889,7 +1906,8 @@ function vipgoci_run() {
18891906
vipgoci_lint_scan_commit(
18901907
$options,
18911908
$results['issues'],
1892-
$results['stats'][ VIPGOCI_STATS_LINT ]
1909+
$results['stats'][ VIPGOCI_STATS_LINT ],
1910+
$results[ VIPGOCI_SKIPPED_FILES ]
18931911
);
18941912
}
18951913

@@ -1902,7 +1920,8 @@ function vipgoci_run() {
19021920
vipgoci_phpcs_scan_commit(
19031921
$options,
19041922
$results['issues'],
1905-
$results['stats'][ VIPGOCI_STATS_PHPCS ]
1923+
$results['stats'][ VIPGOCI_STATS_PHPCS ],
1924+
$results[ VIPGOCI_SKIPPED_FILES ]
19061925
);
19071926
}
19081927

0 commit comments

Comments
 (0)