Skip to content

Commit f586d45

Browse files
pstaabppstaabp
committed
Improvements on the PGcritic
Added extra features to the critic.
1 parent 7a221d9 commit f586d45

File tree

2 files changed

+126
-38
lines changed

2 files changed

+126
-38
lines changed

‎bin/pg-critic.pl‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ =head1 SYNOPSIS
88
99
pg-critic.pl [options] file1 file2 ...
1010
11+
Options:
12+
13+
-s|--score Give a score for each file.
14+
-f|--format Format of the output. Default ('text') is a plain text listing of the filename
15+
and the score. 'JSON' will make a JSON file.
16+
For output format 'JSON', the filename output must also be assigned,
17+
however for 'text', the output is optional.
18+
-o|--output-file Filename for the output. Note: this is required if JSON is the output format.
19+
-d|--details Include the details in the output. (Only used if the format is JSON).
20+
-v|--verbose Increase the verbosity of the output.
21+
-h|--help Show the help message.
22+
1123
=head1 DESCRIPTION
1224
1325
This script analyzes the input files for old/deprecated functions and macros as well
@@ -30,26 +42,42 @@ =head1 OPTIONS
3042
use feature 'say';
3143

3244
use Mojo::File qw(curfile);
33-
use Getopt::Long;
3445
use Mojo::Util qw(dumper);
46+
use Mojo::JSON qw(encode_json);
47+
use Getopt::Long;
48+
use Pod::Usage;
3549

3650
use lib curfile->dirname->dirname . '/lib';
3751

3852
use WeBWorK::PG::PGProblemCritic qw(analyzePGfile);
3953

40-
my $verbose = 0;
41-
my $score = 0;
54+
my ($verbose, $show_score, $details, $show_help) = (0, 1, 0, 0);
55+
my ($format, $filename) = ('text', '');
4256
GetOptions(
43-
"v|verbose" => \$verbose,
44-
's|score' => \$score
57+
's|score' => \$show_score,
58+
'f|format=s' => \$format,
59+
'o|output-file=s' => \$filename,
60+
'd|details' => \$details,
61+
"v|verbose" => \$verbose,
62+
'h|help' => \$show_help
4563
);
64+
pod2usage(2) if $show_help || !$show_score;
4665

4766
die 'arguments must have a list of pg files' unless @ARGV > 0;
67+
die "The output format must be 'text' or 'JSON'" if (scalar(grep { $_ eq $format } qw(text JSON)) == 0);
68+
69+
my $output_file;
70+
unless ($format eq 'text' && $filename eq '') {
71+
die "The output-file is required if using the format: $format" if $filename eq '';
72+
$output_file = Mojo::File->new($filename);
73+
my $dir = $output_file->dirname->realpath;
74+
die "The output directory $dir does not exist or is not a directory" unless -d $dir->to_string;
75+
}
4876

4977
# Give a problem an assessment score:
5078

5179
my $rubric = {
52-
metadata => -5,
80+
metadata => -5, # score for each missing required metadta
5381
good => {
5482
PGML => 20,
5583
solution => 30,
@@ -59,6 +87,17 @@ =head1 OPTIONS
5987
multianswer => 30,
6088
answer_hints => 20,
6189
nicetable => 10,
90+
contexts => { base_n => 10, units => 10, boolean => 10, reaction => 10 },
91+
parsers => { radio_buttons => 10, checkbox_list => 10, radio_multianswer => 10, graph_tool => 10 },
92+
macros => {
93+
random_person => 10,
94+
plots => 10,
95+
tikz => 10,
96+
plotly3D => 10,
97+
latex_image => 10,
98+
scaffold => 10,
99+
answer_hints => 10,
100+
}
62101
},
63102
bad => {
64103
BEGIN_TEXT => -10,
@@ -70,8 +109,8 @@ =head1 OPTIONS
70109
context_texstrings => -5,
71110
multiple_loadmacros => -20,
72111
showPartialCorrect => -5,
73-
old_multiple_choice => -20,
74112
lines_below_enddocument => -5,
113+
macros => { ww_plot => -20, PGchoicemacros => -20 }
75114
},
76115
deprecated_macros => -10
77116
};
@@ -85,13 +124,30 @@ ($prob)
85124
return $score;
86125
}
87126

127+
my @scores;
128+
88129
for (grep { $_ =~ /\.pg$/ } @ARGV) {
89130
say $_ if $verbose;
90-
my $features = analyzePGfile($_);
91-
say dumper $features if $verbose;
92-
if ($score) {
93-
say dumper scoreProblem($features) if $verbose;
131+
my $features = analyzePGfile($_);
132+
my $file_info = { file => $_, score => scoreProblem($features) };
133+
$file_info->{details} = $features if $details;
134+
push(@scores, $file_info);
135+
}
136+
137+
if ($format eq 'text') {
138+
my $output_str = '';
139+
for my $score (@scores) {
140+
$output_str .= "filename: $score->{file}; score: $score->{score}\n";
141+
}
142+
if ($filename eq '') {
143+
say $output_str;
144+
} else {
145+
$output_file->spew($output_str);
146+
say "Results written in text format to $output_file" if $verbose;
94147
}
148+
} elsif ($format eq 'JSON') {
149+
$output_file->spew(encode_json(\@scores));
150+
say "Results written in JSON format to $output_file" if $verbose;
95151
}
96152

97153
1;

‎lib/WeBWorK/PG/PGProblemCritic.pm‎

Lines changed: 59 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
=head1 NAME
33
4-
PGProblemCritic - Parse a PG program and analyze the contents for good and bad features.
4+
PGProblemCritic - Parse a PG program and analyze the contents for positive and negative features.
55
66
=head1 DESCRIPTION
77
@@ -80,17 +80,27 @@ sub analyzePGcode ($code) {
8080
# default flags for presence of features in a PG problem
8181
my $features = {
8282
metadata => { DBsubject => 0, DBchapter => 0, DBsection => 0, KEYWORDS => 0 },
83-
good => {
83+
positive => {
8484
PGML => 0,
8585
solution => 0,
8686
hint => 0,
87-
scaffold => 0,
8887
custom_checker => 0,
8988
multianswer => 0,
90-
answer_hints => 0,
91-
nicetable => 0,
89+
nicetables => 0,
90+
contexts => { BaseN => 0, Units => 0, Boolean => 0, Reaction => 0 },
91+
parsers =>
92+
{ dropdown => 0, RadioButtons => 0, CheckboxList => 0, RadioMultianswer => 0, GraphTool => 0 },
93+
macros => {
94+
randomPerson => 0,
95+
Plots => 0,
96+
PGtikz => 0,
97+
Plotly3D => 0,
98+
PGlateximage => 0,
99+
Scaffold => 0,
100+
AnswerHints => 0,
101+
}
92102
},
93-
bad => {
103+
negative => {
94104
BEGIN_TEXT => 0,
95105
beginproblem => 0,
96106
oldtable => 0,
@@ -100,8 +110,8 @@ sub analyzePGcode ($code) {
100110
context_texstrings => 0,
101111
multiple_loadmacros => 0,
102112
showPartialCorrect => 0,
103-
old_multiple_choice => 0,
104113
lines_below_enddocument => 0,
114+
macros => { PGgraphmacros => 0, PGchoicemacros => 0 }
105115
},
106116
deprecated_macros => [],
107117
macros => []
@@ -129,12 +139,12 @@ sub analyzePGcode ($code) {
129139
# Skip any full-line comments.
130140
next if $line =~ /^\s*#/;
131141

132-
$features->{good}{solution} = 1 if $line =~ /BEGIN_(PGML_)?SOLUTION/;
133-
$features->{good}{hint} = 1 if $line =~ /BEGIN_(PGML_)?HINT/;
142+
$features->{positive}{solution} = 1 if $line =~ /BEGIN_(PGML_)?SOLUTION/;
143+
$features->{positive}{hint} = 1 if $line =~ /BEGIN_(PGML_)?HINT/;
134144

135145
# Analyze the loadMacros info.
136146
if ($line =~ /loadMacros\(/) {
137-
$features->{bad}{multiple_loadmacros} = 1 if $loadmacros_parsed == 1;
147+
$features->{negative}{multiple_loadmacros} = 1 if $loadmacros_parsed == 1;
138148
$loadmacros_parsed = 1;
139149
# Parse the macros, which may be on multiple rows.
140150
my $macros = $line;
@@ -161,7 +171,7 @@ sub analyzePGcode ($code) {
161171
push(@{ $features->{deprecated_macros} }, $macro) if (grep { $macro eq $_ } @$all_deprecated_macros);
162172
}
163173
} elsif ($line =~ /BEGIN_PGML(_SOLUTION|_HINT)?/) {
164-
$features->{good}{PGML} = 1;
174+
$features->{positive}{PGML} = 1;
165175
my @pgml_lines;
166176
while (1) {
167177
$line = shift @pglines;
@@ -170,40 +180,62 @@ sub analyzePGcode ($code) {
170180
}
171181

172182
my $pgml_features = analyzePGMLBlock(@pgml_lines);
173-
$features->{bad}{missing_alt_tag} = 1 if $pgml_features->{missing_alt_tag};
183+
$features->{negative}{missing_alt_tag} = 1 if $pgml_features->{missing_alt_tag};
174184
}
175185

176186
if ($line =~ /ENDDOCUMENT/) { # scan if there are any lines below the ENDDOCUMENT
177187

178188
do {
179189
$line = shift @pglines;
180190
last unless defined($line);
181-
$features->{bad}{lines_below_enddocument} = 1 if $line !~ /^\s*$/;
191+
$features->{negative}{lines_below_enddocument} = 1 if $line !~ /^\s*$/;
182192
} while (defined($line));
183193
}
184194

185-
# Check for bad features.
186-
$features->{bad}{beginproblem} = 1 if $line =~ /beginproblem\(\)/;
187-
$features->{bad}{BEGIN_TEXT} = 1 if $line =~ /(BEGIN_(TEXT|HINT|SOLUTION))|EV[23]/;
188-
$features->{bad}{context_texstrings} = 1 if $line =~ /->(texStrings|normalStrings)/;
195+
# Check for negative features.
196+
$features->{negative}{beginproblem} = 1 if $line =~ /beginproblem\(\)/;
197+
$features->{negative}{BEGIN_TEXT} = 1 if $line =~ /(BEGIN_(TEXT|HINT|SOLUTION))|EV[23]/;
198+
$features->{negative}{context_texstrings} = 1 if $line =~ /->(texStrings|normalStrings)/;
189199
for (qw(num str fun)) {
190-
$features->{bad}{ $_ . '_cmp' } = 1 if $line =~ /${_}_cmp\(/;
200+
$features->{negative}{ $_ . '_cmp' } = 1 if $line =~ /${_}_cmp\(/;
191201
}
192-
$features->{bad}{oldtable} = 1 if $line =~ /BeginTable/i;
193-
$features->{bad}{showPartialCorrect} = 1 if $line =~ /\$showPartialCorrectAnswers\s=\s1/;
194-
$features->{bad}{old_multiple_choice} = 1
202+
$features->{negative}{oldtable} = 1 if $line =~ /BeginTable/i;
203+
$features->{negative}{showPartialCorrect} = 1 if $line =~ /\$showPartialCorrectAnswers\s=\s1/;
204+
$features->{negative}{macros}{PGgraphmacros} = 1 if $line =~ /init_graph\(/;
205+
$features->{negative}{macros}{PGchoicemacros} = 1
195206
if $line =~ /new_checkbox_multiple_choice/
196207
|| $line =~ /new_match_list/
197208
|| $line =~ /new_select_list/
198209
|| $line =~ /new_multiple_choice/
199210
|| $line =~ /qa\s\(/;
200211

201-
# check for good features
202-
$features->{good}{scaffold} = 1 if $line =~ /Scaffold::Begin/;
203-
$features->{good}{answer_hints} = 1 if $line =~ /AnswerHints/;
204-
$features->{good}{multianswer} = 1 if $line =~ /MultiAnswer/;
205-
$features->{good}{custom_checker} = 1 if $line =~ /checker =>/;
206-
$features->{good}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/;
212+
# check for positive features
213+
# macros:
214+
$features->{positive}{macros}{Scaffold} = 1 if $line =~ /Scaffold::Begin/;
215+
$features->{positive}{macros}{Plots} = 1 if $line =~ /Plot\(/;
216+
$features->{positive}{macros}{Plotly3D} = 1 if $line =~ /Graph3D\(/;
217+
$features->{positive}{macros}{PGtikz} = 1 if $line =~ /createTikZImage\(/;
218+
$features->{positive}{macros}{AnswerHints} = 1 if $line =~ /AnswerHints/;
219+
$features->{positive}{macros}{randomPerson} = 1 if $line =~ /randomPerson\(/ || $line =~ /randomLastName\(/;
220+
$features->{positive}{macros}{PGlateximage} = 1 if $line =~ /createLaTeXImage\(/;
221+
222+
# contexts:
223+
224+
$features->{positive}{contexts}{Units} = 1 if $line =~ /Context\(['"]Units['"]\)/;
225+
$features->{positive}{contexts}{BaseN} = 1 if $line =~ /Context\(['"](Limited)?BaseN['"]\)/;
226+
$features->{positive}{contexts}{Boolean} = 1 if $line =~ /Context\(['"]Boolean['"]\)/;
227+
$features->{positive}{contexts}{Reaction} = 1 if $line =~ /Context\(['"]Reaction['"]\)/;
228+
229+
# parsers:
230+
$features->{positive}{parsers}{PopUp} = 1 if $line =~ /DropDown\(/;
231+
$features->{positive}{parsers}{RadioButtons} = 1 if $line =~ /RadioButtons\(/;
232+
$features->{positive}{parsers}{CheckboxList} = 1 if $line =~ /CheckboxList\(/;
233+
$features->{positive}{parsers}{GraphTool} = 1 if $line =~ /GraphTool\(/;
234+
235+
# other:
236+
$features->{positive}{multianswer} = 1 if $line =~ /MultiAnswer/;
237+
$features->{positive}{custom_checker} = 1 if $line =~ /checker\s*=>/;
238+
$features->{positive}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/;
207239

208240
}
209241
return $features;

0 commit comments

Comments
 (0)