From 80b3625f04c31f2c309db1fa16805395fbec3858 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 20 Jun 2025 12:08:31 -0700 Subject: [PATCH 1/5] Initial checkin of PG critic script. --- bin/pg-critic.pl | 97 +++++++++++ lib/WeBWorK/PG/PGProblemCritic.pm | 274 ++++++++++++++++++++++++++++++ 2 files changed, 371 insertions(+) create mode 100644 bin/pg-critic.pl create mode 100644 lib/WeBWorK/PG/PGProblemCritic.pm diff --git a/bin/pg-critic.pl b/bin/pg-critic.pl new file mode 100644 index 000000000..4f4908ad6 --- /dev/null +++ b/bin/pg-critic.pl @@ -0,0 +1,97 @@ +#!/usr/bin/env perl + +=head1 NAME + +pg-critic.pl -- Analyze a pg file for use of old and current methods. + +=head1 SYNOPSIS + + pg-critic.pl [options] file1 file2 ... + +=head1 DESCRIPTION + +This script analyzes the input files for old/deprecated functions and macros as well +as features for current best practices features. + +See L for details on what features are determined presence. + +=head1 OPTIONS + +The option C<-v> or C<--verbose> gives more information (on STDOUT) as the +script is run. + +The option C<-s> or C<--score> will return a score for each given PG problem. + +=cut + +use strict; +use warnings; +use experimental 'signatures'; +use feature 'say'; + +use Mojo::File qw(curfile); +use Getopt::Long; +use Data::Dumper; + +use lib curfile->dirname->dirname . '/lib'; + +use WeBWorK::PG::PGProblemCritic qw(analyzePGfile); + +my $verbose = 0; +my $score = 0; +GetOptions( + "v|verbose" => \$verbose, + 's|score' => \$score +); + +die 'arguments must have a list of pg files' unless @ARGV > 0; + +# Give a problem an assessment score: + +my $rubric = { + metadata => -5, + good => { + PGML => 20, + solution => 30, + hint => 10, + scaffold => 50, + custom_checker => 50, + multianswer => 30, + answer_hints => 20, + nicetable => 10, + }, + bad => { + BEGIN_TEXT => -10, + beginproblem => -5, + oldtable => -25, + num_cmp => -75, + str_cmp => -75, + fun_cmp => -75, + context_texstrings => -5, + multiple_loadmacros => -20, + showPartialCorrect => -5, + old_multiple_choice => -20, + lines_below_enddocument => -5, + }, + deprecated_macros => -10 +}; + +sub scoreProblem ($prob) { + my $score = 0; + $score += (1 - $prob->{metadata}{$_}) * $rubric->{metadata} for (keys %{ $prob->{metadata} }); + $score += $prob->{good}{$_} * $rubric->{good}{$_} for (keys %{ $prob->{good} }); + $score += $prob->{bad}{$_} * $rubric->{bad}{$_} for (keys %{ $prob->{bad} }); + $score += $rubric->{deprecated_macros} for (@{ $prob->{deprecated_macros} }); + return $score; +} + +for (grep { $_ =~ /\.pg$/ } @ARGV) { + say $_ if $verbose; + my $features = analyzePGfile($_); + # print Dumper $features if $verbose; + if ($score) { + print Dumper scoreProblem($features) if $verbose; + } +} + +1; diff --git a/lib/WeBWorK/PG/PGProblemCritic.pm b/lib/WeBWorK/PG/PGProblemCritic.pm new file mode 100644 index 000000000..87436465b --- /dev/null +++ b/lib/WeBWorK/PG/PGProblemCritic.pm @@ -0,0 +1,274 @@ +package WeBWorK::PG::PGProblemCritic; +use parent qw(Exporter); + +use strict; +use warnings; +use experimental 'signatures'; +use feature 'say'; + +use Mojo::File qw(curfile); +use Data::Dumper; + +our @EXPORT_OK = qw(analyzePGfile analyzePGcode getDeprecatedMacros); + +=head1 NAME + +PGProblemCritic - Parse a PG program and analyze the contents for good and bad features. + +=head1 DESCRIPTION + +Analyze a pg file for use of old and current methods. + +=over + +=item C: a list of the macros that the problem uses that is in the C +folder. + +=item Positive features: + +=over 10 + +=item Uses PGML + +=item Provides a solution + +=item Provides a hint + +=item Uses Scaffolds + +=item Uses a custom checker + +=item Uses a multianswer + +=item Uses answer hints + +=item Uses nicetables + +=back + +=item Old and deprecated features + +=over 10 + +=item Use of BEGIN_TEXT/END_TEXT + +=item Include the C + +=item Include old tables (for example from C) + +=item The use of C, C and C in lieu of using MathObjects + +=item Including C<< Context()->TeXStrings >> + +=item Calling C more than once. + +=item Using the line C< $showPartialCorrectAnswers = 1 > which is the default behavior and thus unnecessary. + +=item Using methods from C + +=item Inlcuding code or other text below the C line indicating the end of the problem. + +=back + +=back + + +=cut + +sub analyzePGcode ($code) { + # default flags for presence of features in a PG problem + my $features = { + metadata => { DBsubject => 0, DBchapter => 0, DBsection => 0, KEYWORDS => 0 }, + good => { + PGML => 0, + solution => 0, + hint => 0, + scaffold => 0, + custom_checker => 0, + multianswer => 0, + answer_hints => 0, + nicetable => 0, + }, + bad => { + BEGIN_TEXT => 0, + beginproblem => 0, + oldtable => 0, + num_cmp => 0, + str_cmp => 0, + fun_cmp => 0, + context_texstrings => 0, + multiple_loadmacros => 0, + showPartialCorrect => 0, + old_multiple_choice => 0, + lines_below_enddocument => 0, + }, + deprecated_macros => [], + macros => [] + }; + + # Get a list of all deprecated macros. + my $all_deprecated_macros = getDeprecatedMacros(curfile->dirname->dirname->dirname->dirname); + + # determine if the loadMacros has been parsed. + my $loadmacros_parsed = 0; + + my @pglines = split /\n/, $code; + my $line = ''; + while (1) { + $line = shift @pglines; + # print Dumper $line; + last unless defined($line); # end of the file. + next if $line =~ /^\s*$/; # skip any blank lines. + + # Determine if some of the metadata tags are present. + for (qw(DBsubject DBchapter DBsection KEYWORDS)) { + $features->{metadata}{$_} = 1 if $line =~ /$_\(/i; + } + + # Skip any full-line comments. + next if $line =~ /^\s*#/; + + $features->{good}{solution} = 1 if $line =~ /BEGIN_(PGML_)?SOLUTION/; + $features->{good}{hint} = 1 if $line =~ /BEGIN_(PGML_)?HINT/; + + # Analyze the loadMacros info. + if ($line =~ /loadMacros\(/) { + $features->{bad}{multiple_loadmacros} = 1 if $loadmacros_parsed == 1; + $loadmacros_parsed = 1; + # Parse the macros, which may be on multiple rows. + my $macros = $line; + while ($line && $line !~ /\);\s*$/) { + $line = shift @pglines; + + # Strip any comments at the end of lines. + $line =~ s/(.*)#.*/$1/; + $macros .= $line; + } + # Split by commas and pull out the quotes. + # TODO: handle cases with loadMacros(qw/macro1.pl macro2.pl/); + my @macros = map {s/['"\s]//gr} split(/\s*,\s*/, $macros =~ s/loadMacros\((.*)\)\;$/$1/r); + $features->{macros} = \@macros; + for my $macro (@macros) { + push(@{ $features->{deprecated_macros} }, $macro) if (grep { $macro eq $_ } @$all_deprecated_macros); + } + } elsif ($line =~ /BEGIN_PGML(_SOLUTION|_HINT)?/) { + $features->{good}{PGML} = 1; + my @pgml_lines; + while (1) { + $line = shift @pglines; + last if $line =~ /END_PGML(_SOLUTON|_HINT)?/; + push(@pgml_lines, $line); + } + + my $pgml_features = analyzePGMLBlock(@pgml_lines); + $features->{bad}{missing_alt_tag} = 1 if $pgml_features->{missing_alt_tag}; + } + + if ($line =~ /ENDDOCUMENT/) { # scan if there are any lines below the ENDDOCUMENT + + do { + $line = shift @pglines; + last unless defined($line); + $features->{bad}{lines_below_enddocument} = 1 if $line !~ /^\s*$/; + } while (defined($line)); + } + + # Check for bad features. + $features->{bad}{beginproblem} = 1 if $line =~ /beginproblem\(\)/; + $features->{bad}{BEGIN_TEXT} = 1 if $line =~ /(BEGIN_(TEXT|HINT|SOLUTION))|EV[23]/; + $features->{bad}{context_texstrings} = 1 if $line =~ /->(texStrings|normalStrings)/; + for (qw(num str fun)) { + $features->{bad}{ $_ . '_cmp' } = 1 if $line =~ /${_}_cmp\(/; + } + $features->{bad}{oldtable} = 1 if $line =~ /BeginTable/i; + $features->{bad}{showPartialCorrect} = 1 if $line =~ /\$showPartialCorrectAnswers\s=\s1/; + $features->{bad}{old_multiple_choice} = 1 + if $line =~ /new_checkbox_multiple_choice/ + || $line =~ /new_match_list/ + || $line =~ /new_select_list/ + || $line =~ /new_multiple_choice/ + || $line =~ /qa\s\(/; + + # check for good features + $features->{good}{scaffold} = 1 if $line =~ /Scaffold::Begin/; + $features->{good}{answer_hints} = 1 if $line =~ /AnswerHints/; + $features->{good}{multianswer} = 1 if $line =~ /MultiAnswer/; + $features->{good}{custom_checker} = 1 if $line =~ /checker =>/; + $features->{good}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/; + + } + return $features; +} + +# Return a list of the macro filenames in the 'macros/deprecated' directory. +sub getDeprecatedMacros ($pgroot) { + return Mojo::File->new($pgroot)->child('macros/deprecated')->list->map(sub { $_->basename })->to_array; +} + +sub analyzePGfile ($file) { + my $path = Mojo::File->new($file); + die "The file: $file does not exist or is not readable" unless -r $path; + + return analyzePGcode($path->slurp); +} + +# Parse a string that is a function in the form of "funct($arg1, $arg2, ..., param1 => val1, param2 => val2 , ...)" +# A hashref of the form {_args = [$arg1, $arg2, ...], param1 => val1, param2 => val2} is returned. + +sub parseFunctionString($string) { + + my ($funct, $args); + if ($string =~ /(\w+)\(\s*(.*)\)/) { + ($funct, $args) = ($1, $2); + } else { + return (); + } + + my @args = split(/,\s/, $args); + + my %params = (_name => $funct, _args => []); + for (@args) { + if ($_ !~ /=>/) { + push(@{ $params{_args} }, $_); + } else { + if ($_ =~ /(\w+)\s*=>\s*["']?([^"]*)["']?/) { + my ($key, $value) = ($1, $2); + $params{$key} = $value; + } + } + } + return %params; +} + +# Perform some analysis of a PGML block. + +sub analyzePGMLBlock(@lines) { + my $pgml_features = {}; + + while (1) { + my $line = shift @lines; + last unless defined($line); + + # If there is a perl block analyze [@ @] + if ($line =~ /\[@/) { + my $perl_line = $line; + while ($perl_line !~ /@\]/) { + $line = shift @lines; + $perl_line .= $line; + } + my ($perlcode) = $perl_line =~ /\[@\s*(.*)\s*@\]/; + + my %funct_info = parseFunctionString($perlcode); + if (%funct_info && $funct_info{_name} =~ /image/) { + if (defined($funct_info{extra_html_tags}) && $funct_info{extra_html_tags} !~ /alt/) { + $pgml_features->{missing_alt_tag} = 1; + } + } + + } elsif (my ($alt_text) = $line =~ /\[!(.*)!\]/) { + $pgml_features->{missing_alt_tag} = 1 if $alt_text =~ /^\s$/; + } + + } + return $pgml_features; +} From 3ef45d5b927ffad068ea267ad75790c73a8ac5cd Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 20 Jun 2025 12:55:52 -0700 Subject: [PATCH 2/5] Update the executable bit of the file. --- bin/pg-critic.pl | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 bin/pg-critic.pl diff --git a/bin/pg-critic.pl b/bin/pg-critic.pl old mode 100644 new mode 100755 From 7a221d90fc41114bb1277fdcf26cff37c27f7b44 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 20 Jun 2025 13:54:10 -0700 Subject: [PATCH 3/5] Handle loadMacros arguments in the form q{ } and related. --- bin/pg-critic.pl | 6 ++--- lib/WeBWorK/PG/PGProblemCritic.pm | 39 +++++++++++++++++++------------ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/bin/pg-critic.pl b/bin/pg-critic.pl index 4f4908ad6..c228dee54 100755 --- a/bin/pg-critic.pl +++ b/bin/pg-critic.pl @@ -31,7 +31,7 @@ =head1 OPTIONS use Mojo::File qw(curfile); use Getopt::Long; -use Data::Dumper; +use Mojo::Util qw(dumper); use lib curfile->dirname->dirname . '/lib'; @@ -88,9 +88,9 @@ ($prob) for (grep { $_ =~ /\.pg$/ } @ARGV) { say $_ if $verbose; my $features = analyzePGfile($_); - # print Dumper $features if $verbose; + say dumper $features if $verbose; if ($score) { - print Dumper scoreProblem($features) if $verbose; + say dumper scoreProblem($features) if $verbose; } } diff --git a/lib/WeBWorK/PG/PGProblemCritic.pm b/lib/WeBWorK/PG/PGProblemCritic.pm index 87436465b..8026634f8 100644 --- a/lib/WeBWorK/PG/PGProblemCritic.pm +++ b/lib/WeBWorK/PG/PGProblemCritic.pm @@ -1,15 +1,3 @@ -package WeBWorK::PG::PGProblemCritic; -use parent qw(Exporter); - -use strict; -use warnings; -use experimental 'signatures'; -use feature 'say'; - -use Mojo::File qw(curfile); -use Data::Dumper; - -our @EXPORT_OK = qw(analyzePGfile analyzePGcode getDeprecatedMacros); =head1 NAME @@ -75,6 +63,19 @@ folder. =cut +package WeBWorK::PG::PGProblemCritic; +use parent qw(Exporter); + +use strict; +use warnings; +use experimental 'signatures'; +use feature 'say'; + +use Mojo::File qw(curfile); +use Mojo::Util qw(dumper); + +our @EXPORT_OK = qw(analyzePGfile analyzePGcode getDeprecatedMacros); + sub analyzePGcode ($code) { # default flags for presence of features in a PG problem my $features = { @@ -144,9 +145,17 @@ sub analyzePGcode ($code) { $line =~ s/(.*)#.*/$1/; $macros .= $line; } - # Split by commas and pull out the quotes. - # TODO: handle cases with loadMacros(qw/macro1.pl macro2.pl/); - my @macros = map {s/['"\s]//gr} split(/\s*,\s*/, $macros =~ s/loadMacros\((.*)\)\;$/$1/r); + + $macros =~ s/^\s*loadMacros\(\s*(.*)\s*\);\s*$/$1/; + my @macros; + # if the arguments of loadMacros is q[qw] form, handle this. + if ($macros =~ /^q[qw]?[\(\[\{\/](.*)[\)\]\/\}]$/) { + $macros =~ s/^q[qw]?[\(\[\{\/](.*)[\)\]\/\}]$/$1/; + @macros = grep { $_ ne '' } split(/\s+/, $macros); + } else { # arguments are strings separated by commas. + @macros = map {s/['"\s]//gr} split(/\s*,\s*/, $macros =~ s/loadMacros\((.*)\)\;$/$1/r); + } + $features->{macros} = \@macros; for my $macro (@macros) { push(@{ $features->{deprecated_macros} }, $macro) if (grep { $macro eq $_ } @$all_deprecated_macros); From f586d4590b94289d6fa88362d94cf42b9d4da8f3 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Fri, 27 Jun 2025 16:59:27 -0400 Subject: [PATCH 4/5] Improvements on the PGcritic Added extra features to the critic. --- bin/pg-critic.pl | 78 ++++++++++++++++++++++++---- lib/WeBWorK/PG/PGProblemCritic.pm | 86 +++++++++++++++++++++---------- 2 files changed, 126 insertions(+), 38 deletions(-) diff --git a/bin/pg-critic.pl b/bin/pg-critic.pl index c228dee54..2fc6b855f 100755 --- a/bin/pg-critic.pl +++ b/bin/pg-critic.pl @@ -8,6 +8,18 @@ =head1 SYNOPSIS pg-critic.pl [options] file1 file2 ... +Options: + + -s|--score Give a score for each file. + -f|--format Format of the output. Default ('text') is a plain text listing of the filename + and the score. 'JSON' will make a JSON file. + For output format 'JSON', the filename output must also be assigned, + however for 'text', the output is optional. + -o|--output-file Filename for the output. Note: this is required if JSON is the output format. + -d|--details Include the details in the output. (Only used if the format is JSON). + -v|--verbose Increase the verbosity of the output. + -h|--help Show the help message. + =head1 DESCRIPTION This script analyzes the input files for old/deprecated functions and macros as well @@ -30,26 +42,42 @@ =head1 OPTIONS use feature 'say'; use Mojo::File qw(curfile); -use Getopt::Long; use Mojo::Util qw(dumper); +use Mojo::JSON qw(encode_json); +use Getopt::Long; +use Pod::Usage; use lib curfile->dirname->dirname . '/lib'; use WeBWorK::PG::PGProblemCritic qw(analyzePGfile); -my $verbose = 0; -my $score = 0; +my ($verbose, $show_score, $details, $show_help) = (0, 1, 0, 0); +my ($format, $filename) = ('text', ''); GetOptions( - "v|verbose" => \$verbose, - 's|score' => \$score + 's|score' => \$show_score, + 'f|format=s' => \$format, + 'o|output-file=s' => \$filename, + 'd|details' => \$details, + "v|verbose" => \$verbose, + 'h|help' => \$show_help ); +pod2usage(2) if $show_help || !$show_score; die 'arguments must have a list of pg files' unless @ARGV > 0; +die "The output format must be 'text' or 'JSON'" if (scalar(grep { $_ eq $format } qw(text JSON)) == 0); + +my $output_file; +unless ($format eq 'text' && $filename eq '') { + die "The output-file is required if using the format: $format" if $filename eq ''; + $output_file = Mojo::File->new($filename); + my $dir = $output_file->dirname->realpath; + die "The output directory $dir does not exist or is not a directory" unless -d $dir->to_string; +} # Give a problem an assessment score: my $rubric = { - metadata => -5, + metadata => -5, # score for each missing required metadta good => { PGML => 20, solution => 30, @@ -59,6 +87,17 @@ =head1 OPTIONS multianswer => 30, answer_hints => 20, nicetable => 10, + contexts => { base_n => 10, units => 10, boolean => 10, reaction => 10 }, + parsers => { radio_buttons => 10, checkbox_list => 10, radio_multianswer => 10, graph_tool => 10 }, + macros => { + random_person => 10, + plots => 10, + tikz => 10, + plotly3D => 10, + latex_image => 10, + scaffold => 10, + answer_hints => 10, + } }, bad => { BEGIN_TEXT => -10, @@ -70,8 +109,8 @@ =head1 OPTIONS context_texstrings => -5, multiple_loadmacros => -20, showPartialCorrect => -5, - old_multiple_choice => -20, lines_below_enddocument => -5, + macros => { ww_plot => -20, PGchoicemacros => -20 } }, deprecated_macros => -10 }; @@ -85,13 +124,30 @@ ($prob) return $score; } +my @scores; + for (grep { $_ =~ /\.pg$/ } @ARGV) { say $_ if $verbose; - my $features = analyzePGfile($_); - say dumper $features if $verbose; - if ($score) { - say dumper scoreProblem($features) if $verbose; + my $features = analyzePGfile($_); + my $file_info = { file => $_, score => scoreProblem($features) }; + $file_info->{details} = $features if $details; + push(@scores, $file_info); +} + +if ($format eq 'text') { + my $output_str = ''; + for my $score (@scores) { + $output_str .= "filename: $score->{file}; score: $score->{score}\n"; + } + if ($filename eq '') { + say $output_str; + } else { + $output_file->spew($output_str); + say "Results written in text format to $output_file" if $verbose; } +} elsif ($format eq 'JSON') { + $output_file->spew(encode_json(\@scores)); + say "Results written in JSON format to $output_file" if $verbose; } 1; diff --git a/lib/WeBWorK/PG/PGProblemCritic.pm b/lib/WeBWorK/PG/PGProblemCritic.pm index 8026634f8..d4833127b 100644 --- a/lib/WeBWorK/PG/PGProblemCritic.pm +++ b/lib/WeBWorK/PG/PGProblemCritic.pm @@ -1,7 +1,7 @@ =head1 NAME -PGProblemCritic - Parse a PG program and analyze the contents for good and bad features. +PGProblemCritic - Parse a PG program and analyze the contents for positive and negative features. =head1 DESCRIPTION @@ -80,17 +80,27 @@ sub analyzePGcode ($code) { # default flags for presence of features in a PG problem my $features = { metadata => { DBsubject => 0, DBchapter => 0, DBsection => 0, KEYWORDS => 0 }, - good => { + positive => { PGML => 0, solution => 0, hint => 0, - scaffold => 0, custom_checker => 0, multianswer => 0, - answer_hints => 0, - nicetable => 0, + nicetables => 0, + contexts => { BaseN => 0, Units => 0, Boolean => 0, Reaction => 0 }, + parsers => + { dropdown => 0, RadioButtons => 0, CheckboxList => 0, RadioMultianswer => 0, GraphTool => 0 }, + macros => { + randomPerson => 0, + Plots => 0, + PGtikz => 0, + Plotly3D => 0, + PGlateximage => 0, + Scaffold => 0, + AnswerHints => 0, + } }, - bad => { + negative => { BEGIN_TEXT => 0, beginproblem => 0, oldtable => 0, @@ -100,8 +110,8 @@ sub analyzePGcode ($code) { context_texstrings => 0, multiple_loadmacros => 0, showPartialCorrect => 0, - old_multiple_choice => 0, lines_below_enddocument => 0, + macros => { PGgraphmacros => 0, PGchoicemacros => 0 } }, deprecated_macros => [], macros => [] @@ -129,12 +139,12 @@ sub analyzePGcode ($code) { # Skip any full-line comments. next if $line =~ /^\s*#/; - $features->{good}{solution} = 1 if $line =~ /BEGIN_(PGML_)?SOLUTION/; - $features->{good}{hint} = 1 if $line =~ /BEGIN_(PGML_)?HINT/; + $features->{positive}{solution} = 1 if $line =~ /BEGIN_(PGML_)?SOLUTION/; + $features->{positive}{hint} = 1 if $line =~ /BEGIN_(PGML_)?HINT/; # Analyze the loadMacros info. if ($line =~ /loadMacros\(/) { - $features->{bad}{multiple_loadmacros} = 1 if $loadmacros_parsed == 1; + $features->{negative}{multiple_loadmacros} = 1 if $loadmacros_parsed == 1; $loadmacros_parsed = 1; # Parse the macros, which may be on multiple rows. my $macros = $line; @@ -161,7 +171,7 @@ sub analyzePGcode ($code) { push(@{ $features->{deprecated_macros} }, $macro) if (grep { $macro eq $_ } @$all_deprecated_macros); } } elsif ($line =~ /BEGIN_PGML(_SOLUTION|_HINT)?/) { - $features->{good}{PGML} = 1; + $features->{positive}{PGML} = 1; my @pgml_lines; while (1) { $line = shift @pglines; @@ -170,7 +180,7 @@ sub analyzePGcode ($code) { } my $pgml_features = analyzePGMLBlock(@pgml_lines); - $features->{bad}{missing_alt_tag} = 1 if $pgml_features->{missing_alt_tag}; + $features->{negative}{missing_alt_tag} = 1 if $pgml_features->{missing_alt_tag}; } if ($line =~ /ENDDOCUMENT/) { # scan if there are any lines below the ENDDOCUMENT @@ -178,32 +188,54 @@ sub analyzePGcode ($code) { do { $line = shift @pglines; last unless defined($line); - $features->{bad}{lines_below_enddocument} = 1 if $line !~ /^\s*$/; + $features->{negative}{lines_below_enddocument} = 1 if $line !~ /^\s*$/; } while (defined($line)); } - # Check for bad features. - $features->{bad}{beginproblem} = 1 if $line =~ /beginproblem\(\)/; - $features->{bad}{BEGIN_TEXT} = 1 if $line =~ /(BEGIN_(TEXT|HINT|SOLUTION))|EV[23]/; - $features->{bad}{context_texstrings} = 1 if $line =~ /->(texStrings|normalStrings)/; + # Check for negative features. + $features->{negative}{beginproblem} = 1 if $line =~ /beginproblem\(\)/; + $features->{negative}{BEGIN_TEXT} = 1 if $line =~ /(BEGIN_(TEXT|HINT|SOLUTION))|EV[23]/; + $features->{negative}{context_texstrings} = 1 if $line =~ /->(texStrings|normalStrings)/; for (qw(num str fun)) { - $features->{bad}{ $_ . '_cmp' } = 1 if $line =~ /${_}_cmp\(/; + $features->{negative}{ $_ . '_cmp' } = 1 if $line =~ /${_}_cmp\(/; } - $features->{bad}{oldtable} = 1 if $line =~ /BeginTable/i; - $features->{bad}{showPartialCorrect} = 1 if $line =~ /\$showPartialCorrectAnswers\s=\s1/; - $features->{bad}{old_multiple_choice} = 1 + $features->{negative}{oldtable} = 1 if $line =~ /BeginTable/i; + $features->{negative}{showPartialCorrect} = 1 if $line =~ /\$showPartialCorrectAnswers\s=\s1/; + $features->{negative}{macros}{PGgraphmacros} = 1 if $line =~ /init_graph\(/; + $features->{negative}{macros}{PGchoicemacros} = 1 if $line =~ /new_checkbox_multiple_choice/ || $line =~ /new_match_list/ || $line =~ /new_select_list/ || $line =~ /new_multiple_choice/ || $line =~ /qa\s\(/; - # check for good features - $features->{good}{scaffold} = 1 if $line =~ /Scaffold::Begin/; - $features->{good}{answer_hints} = 1 if $line =~ /AnswerHints/; - $features->{good}{multianswer} = 1 if $line =~ /MultiAnswer/; - $features->{good}{custom_checker} = 1 if $line =~ /checker =>/; - $features->{good}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/; + # check for positive features + # macros: + $features->{positive}{macros}{Scaffold} = 1 if $line =~ /Scaffold::Begin/; + $features->{positive}{macros}{Plots} = 1 if $line =~ /Plot\(/; + $features->{positive}{macros}{Plotly3D} = 1 if $line =~ /Graph3D\(/; + $features->{positive}{macros}{PGtikz} = 1 if $line =~ /createTikZImage\(/; + $features->{positive}{macros}{AnswerHints} = 1 if $line =~ /AnswerHints/; + $features->{positive}{macros}{randomPerson} = 1 if $line =~ /randomPerson\(/ || $line =~ /randomLastName\(/; + $features->{positive}{macros}{PGlateximage} = 1 if $line =~ /createLaTeXImage\(/; + + # contexts: + + $features->{positive}{contexts}{Units} = 1 if $line =~ /Context\(['"]Units['"]\)/; + $features->{positive}{contexts}{BaseN} = 1 if $line =~ /Context\(['"](Limited)?BaseN['"]\)/; + $features->{positive}{contexts}{Boolean} = 1 if $line =~ /Context\(['"]Boolean['"]\)/; + $features->{positive}{contexts}{Reaction} = 1 if $line =~ /Context\(['"]Reaction['"]\)/; + + # parsers: + $features->{positive}{parsers}{PopUp} = 1 if $line =~ /DropDown\(/; + $features->{positive}{parsers}{RadioButtons} = 1 if $line =~ /RadioButtons\(/; + $features->{positive}{parsers}{CheckboxList} = 1 if $line =~ /CheckboxList\(/; + $features->{positive}{parsers}{GraphTool} = 1 if $line =~ /GraphTool\(/; + + # other: + $features->{positive}{multianswer} = 1 if $line =~ /MultiAnswer/; + $features->{positive}{custom_checker} = 1 if $line =~ /checker\s*=>/; + $features->{positive}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/; } return $features; From 4702f4ca828eb09104673b2e61c38555c885c550 Mon Sep 17 00:00:00 2001 From: Peter Staab Date: Tue, 8 Jul 2025 14:06:12 -0400 Subject: [PATCH 5/5] Add randomness to the list of criteria to be checked. Also updated the POD --- lib/WeBWorK/PG/PGProblemCritic.pm | 46 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/WeBWorK/PG/PGProblemCritic.pm b/lib/WeBWorK/PG/PGProblemCritic.pm index d4833127b..f637d36f6 100644 --- a/lib/WeBWorK/PG/PGProblemCritic.pm +++ b/lib/WeBWorK/PG/PGProblemCritic.pm @@ -9,52 +9,54 @@ Analyze a pg file for use of old and current methods. =over -=item C: a list of the macros that the problem uses that is in the C +=item * C: a list of the macros that the problem uses that is in the C folder. -=item Positive features: +=item * Positive features: -=over 10 +=over + +=item * Uses PGML -=item Uses PGML +=item * Provides a solution -=item Provides a solution +=item * Provides a hint -=item Provides a hint +=item * Uses Scaffolds -=item Uses Scaffolds +=item * Uses a custom checker -=item Uses a custom checker +=item * Uses a multianswer -=item Uses a multianswer +=item * Uses answer hints -=item Uses answer hints +=item * Uses nicetables -=item Uses nicetables +=item * Uses randomness =back =item Old and deprecated features -=over 10 +=over -=item Use of BEGIN_TEXT/END_TEXT +=item * Use of BEGIN_TEXT/END_TEXT -=item Include the C +=item * Include the C -=item Include old tables (for example from C) +=item * Include old tables (for example from C) -=item The use of C, C and C in lieu of using MathObjects +=item * The use of C, C and C in lieu of using MathObjects -=item Including C<< Context()->TeXStrings >> +=item * Including C<< Context()->TeXStrings >> -=item Calling C more than once. +=item * Calling C more than once. -=item Using the line C< $showPartialCorrectAnswers = 1 > which is the default behavior and thus unnecessary. +=item * Using the line C< $showPartialCorrectAnswers = 1 > which is the default behavior and thus unnecessary. -=item Using methods from C +=item * Using methods from C -=item Inlcuding code or other text below the C line indicating the end of the problem. +=item * Inlcuding code or other text below the C line indicating the end of the problem. =back @@ -87,6 +89,7 @@ sub analyzePGcode ($code) { custom_checker => 0, multianswer => 0, nicetables => 0, + randomness => 0, contexts => { BaseN => 0, Units => 0, Boolean => 0, Reaction => 0 }, parsers => { dropdown => 0, RadioButtons => 0, CheckboxList => 0, RadioMultianswer => 0, GraphTool => 0 }, @@ -236,6 +239,7 @@ sub analyzePGcode ($code) { $features->{positive}{multianswer} = 1 if $line =~ /MultiAnswer/; $features->{positive}{custom_checker} = 1 if $line =~ /checker\s*=>/; $features->{positive}{nicetables} = 1 if $line =~ /DataTable|LayoutTable/; + $features->{positive}{randomness} = 1 if $line =~ /random\(|random_(\w+)\(|list_random\(/; } return $features;