Add the pg-critic problem analyzer to the PGEditor

htdocs/js/PGProblemEditor/pgproblemeditor.js

@@ -231,6 +231,30 @@
 			.catch((err) => showMessage(`Error: ${err?.message ?? err}`));
 	};
 
+	// Send a request to the server to run the PG critic in the CodeMirror editor.
+	const runPGCritic = () => {
+		const request_object = { courseID: document.getElementsByName('courseID')[0]?.value };
+
+		const user = document.getElementsByName('user')[0];
+		if (user) request_object.user = user.value;
+		const sessionKey = document.getElementsByName('key')[0];
+		if (sessionKey) request_object.key = sessionKey.value;
+
+		request_object.rpc_command = 'runPGCritic';
+		request_object.pgCode =
+			webworkConfig?.pgCodeMirror?.source ?? document.getElementById('problemContents')?.value ?? '';
+
+		fetch(webserviceURL, { method: 'post', mode: 'same-origin', body: new URLSearchParams(request_object) })
+			.then((response) => response.json())
+			.then((data) => {
+				renderArea.innerHTML = data.result_data.html;
+			})
+			.catch((err) => {
+				console.log(err);
+				showMessage(`Error: ${err?.message ?? err}`);
+			});
+	};
+
 	document.getElementById('take_action')?.addEventListener('click', async (e) => {
 		if (document.getElementById('current_action')?.value === 'format_code') {
 			e.preventDefault();
@@ -240,6 +264,8 @@
 				document.querySelector('input[name="action.format_code"]:checked').value == 'convertCodeToPGML'
 			) {
 				convertCodeToPGML();
+			} else if (document.querySelector('input[name="action.format_code"]:checked').value == 'runPGCritic') {
+				runPGCritic();
 			}
 			return;
 		}

lib/WebworkWebservice.pm

@@ -270,6 +270,7 @@ sub command_permission {
 		putPastAnswer     => 'problem_grader',
 		tidyPGCode        => 'access_instructor_tools',
 		convertCodeToPGML => 'access_instructor_tools',
+		runPGCritic       => 'access_instructor_tools',
 
 		# WebworkWebservice::RenderProblem
 		renderProblem => 'proctor_quiz_login',

lib/WebworkWebservice/ProblemActions.pm

@@ -21,8 +21,9 @@ use warnings;
 
 use Data::Structure::Util qw(unbless);
 
-use WeBWorK::PG::Tidy          qw(pgtidy);
-use WeBWorK::PG::ConvertToPGML qw(convertToPGML);
+use WeBWorK::PG::Tidy            qw(pgtidy);
+use WeBWorK::PG::ConvertToPGML   qw(convertToPGML);
+use WeBWorK::PG::PGProblemCritic qw(analyzePGcode);
 
 sub getUserProblem {
 	my ($invocant, $self, $params) = @_;
@@ -180,4 +181,21 @@ sub convertCodeToPGML {
 
 }
 
+sub runPGCritic {
+	my ($invocant, $self, $params) = @_;
+	my $pg_critic_results = analyzePGcode($params->{pgCode});
+
+	my $html_output = $self->c->render_to_string(
+		template => 'ContentGenerator/Instructor/PGProblemEditor/pg_critic',
+		results  => $pg_critic_results
+	);
+
+	return {
+		ra_out => {
+			html => $html_output
+		},
+		text => 'The script pg-critic has been run successfully.'
+	};
+}
+
 1;

templates/ContentGenerator/Instructor/PGProblemEditor/format_code_form.html.ep

@@ -30,4 +30,18 @@
 			<span class="visually-hidden"><%= maketext('PGML Conversion Help') %></span>
 		</a>
 	</div>
+	<div class="form-check">
+		<%= radio_button 'action.format_code' => 'runPGCritic',
+			id => 'action_format_code_run_pgcritic', class => 'form-check-input'=%>
+		<%= label_for 'action_format_code_run_pgcritic', class => 'form-check-label', begin =%>
+			<%== maketext('Run the PG Critic Analyzer') =%>
+		<% end =%>
+		<a class="help-popup" data-bs-content="<%== maketext('This option runs the PG Critic '
+			. 'code analyzer, which gives suggestions on using more modern PG constructs and  '
+			. 'ensure that you include important features. ') =%>"
+		data-bs-placement="top" data-bs-toggle="popover" role="button">
+			<i aria-hidden="true" class="fas fa-question-circle"></i>
+			<span class="visually-hidden"><%= maketext('PG Critic Help') %></span>
+		</a>
+	</div>
 </div>

templates/ContentGenerator/Instructor/PGProblemEditor/pg_critic.html.ep

@@ -0,0 +1,147 @@
+<div class="m-3" style="overflow: scroll">
+<h2>PG Critic Results</h2>
+
+<h3>Metadata</h3>
+
+<p>The following lists required metadata. If any is missing, the given tag must be filled in.
+	However, make sure that the categories are correct, especially if the problem has been
+	copied.</p>
+
+% sub showIcon { my $show = shift;
+%  return $show ? q!<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-check-square text-success" viewBox="0 0 16 16">
+%  <path d="M14 1a1 1 0 0 1 1 1v12a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2a1 1 0 0 1 1-1zM2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2z"/>
+%  <path d="M10.97 4.97a.75.75 0 0 1 1.071 1.05l-3.992 4.99a.75.75 0 0 1-1.08.02L4.324 8.384a.75.75 0 1 1 1.06-1.06l2.094 2.093 3.473-4.425z"/>
+% </svg>! : q!<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-x-square text-danger" viewBox="0 0 16 16">
+%  <path d="M14 1a1 1 0 0 1 1 1v12a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2a1 1 0 0 1 1-1zM2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2z"/>
+%  <path d="M4.646 4.646a.5.5 0 0 1 .708 0L8 7.293l2.646-2.647a.5.5 0 0 1 .708.708L8.707 8l2.647 2.646a.5.5 0 0 1-.708.708L8 8.707l-2.646 2.647a.5.5 0 0 1-.708-.708L7.293 8 4.646 5.354a.5.5 0 0 1 0-.708"/>
+% </svg>!;
+%}
+
+<table class="table table-bordered">
+<tbody>
+<tr><th>DBsubject</th><td> <%== showIcon($results->{metadata}{DBsection}) %> </td></tr>
+<tr><th>DBchapter</th><td> <%== showIcon($results->{metadata}{DBchapter}) %> </td></tr>
+<tr><th>DBsection</th><td> <%== showIcon($results->{metadata}{DBsection})  %> </td></tr>
+<tr><th>Keywords</th><td> <%== showIcon($results->{metadata}{KEYWORDS})  %> </td></tr>
+</table>
+
+% my $pos = $results->{positive};
+% if ($pos->{PGML} || $pos->{solution} || $pos->{hint}) {
+<h3>Good aspects of this problems are the following</h3>
+% }
+<table class="table table-bordered">
+<tbody>
+% if ($pos->{PGML}) {
+	<tr><th>PGML</th><td>This problem uses PGML, the current preferred way to write problem (text), solution and hint
+		blocks.</td></tr>
+% }
+% if ($pos->{solution}) {
+	<tr><th>Solutions</th><td>This problem has a solution block.  Every problem should have solutions that the
+		student can view after the answer data. </td></tr>
+% }
+% if ($pos->{hint}) {
+	<tr><th>Hints</th><td>This problem has a hint.  This can be helpful for students after attempting the problem
+		a few times (this can be set by the instructor).
+% }
+% if ($pos->{randomness}) {
+	<tr><th>Randomness</th><td>This problem uses randomness.  This is desired to give to a class of students, each
+		of whom may have a different problem. </td>
+% }
+% # list of the positive contexts:
+% my @good_contexts =  grep { $pos->{contexts}{$_} } keys %{$pos->{parsers}};
+% if (@good_contexts) {
+	<tr><th>Modern Contexts</th><td>This problem uses the following modern contexts:
+		<%= join(', ', @good_contexts) %> </td>
+% }
+% my @good_parsers = grep { $pos->{parsers}->{$_} } keys %{$pos->{parsers}};
+% if (@good_parsers) {
+	<tr><th>Modern Parsers</th><td>This problem uses features of the following modern parsers:
+		<%= join(', ', @good_parsers) %> </td>
+% }
+% my @good_macros = grep { $pos->{macros}->{$_} } keys %{$pos->{macros}};
+% if (@good_macros) {
+	<tr><th>Modern Macros</th><td>This problem uses functionality from the following modern macros:
+		<%= join(', ', @good_macros) %> </td>
+% }
+</tbody>
+</table>
+
+
+% if( scalar(@{$results->{deprecated_macros}}) > 0) {
+	<h3>Deprecated Macros</h3>
+	<p>This problem has the following deprecated macros: <%= join(', ',@{$results->{deprecated_macros}} ) %> </p>
+
+	<p>These should be removed from the problem in that these macros will be deleted from PG in a future
+		version.  The functions from these macros may be listed below to help aid in transitioning away from
+		these macros. </p>
+% }
+
+%  my $has_bad_features = 0;
+%  $has_bad_features += $results->{negative}{$_} for (keys %{$results->{negative}});
+
+% if ($has_bad_features || !$pos->{solution}) {
+	<h3>You can improve on the following:</h3>
+	<p> There are features in this problem that contain old or deprecated features.  The following
+		list gives feedback of how the problem can be improved. </p>
+%}
+
+<ul>
+% if ($results->{negative}{BEGIN_TEXT}) {
+	<li>This problem contains older formatting blocks like BEGIN_TEXT.  Consider use PGML.
+		In the <em>Format Code</em> section of the PG Editor, the "Convert to PGML" should be used
+		as a start to get the problem switched.</li>
+%}
+% if ($results->{negative}{beginproblem}) {
+	<li>This problem contains the line <tt>TEXT(beginproblem())</tt>.  This is no longer necessary and should be removed. </li>
+%}
+% if ($results->{negative}{context_texstrings}) {
+	<li>This problem contains  the line <tt>Context()->texStrings;</tt>.  This is no longer necessary and should be removed.  </li>
+%}
+% if ($results->{negative}{oldtable}) {
+	<li>This problem contains the deprecated <tt>begintable</tt> command.  This is not assessible and often cannot be
+		converted to hardcopy.  This table should be written using <tt>nicetables</tt> or a PGML table.   </li>
+%}
+% if ($results->{negative}{showPartialCorrect}) {
+	<li>This problem contains the line <tt>$showPartialCorrectAnswers = 1</tt>. This is enabled by default and needed only
+		if set to 0.</li>
+% }
+% if (!$pos->{solution}) {
+	<li>This problem does not have a solution.  Consider adding one.</li>
+% }
+% if (!$pos->{randomness}) {
+	<li>This problem does not have randomness.  Consider adding variables that take on random values.</li>
+% }
+% if ($results->{negative}{fun_cmp} || $results->{negative}{str_cmp} || $results->{negative}{num_cmp}) {
+	<li>This problem contains the functioins <tt>num_cmp</tt>, <tt>str_cmp</tt> or <tt>fun_cmp</tt>.
+		These are old ways of checking answers.  These should be converted to MathObjects.
+% }
+% if ($results->{negative}{multiple_loadmacros}) {
+	<li>This problem contains two <tt>loadMacros</tt> function call. Combine the function
+		calls and make sure that all macros are needed for your problem. </li>
+% }
+% if ($results->{negative}{macros}{PGchoicemacros}) {
+	<li>This problem contains old versions of multiple choice.  The sample problems
+		<a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoiceCheckbox.html">
+		Multiple Choice with Checkbox</a>, <a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoicePopup.html">
+		Multiple Choice with Popup</a> and <a target="_blank" href="https://openwebwork.github.io/pg-docs/sample-problems/Misc/MultipleChoiceRadio.html">
+		Multiple Choice with Radio Buttons</a> should be examined as well the macros:
+		<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserPopUp.html">parserPopUp.pl</a>,
+		<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserCheckboxList.html">parserCheckboxList.pl</a> and
+		<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/parsers/parserRadioButtons.html">parserRadioButtons.pl</a>.
+
+	 </li>
+% }
+% if ($results->{negative}{macros}{PGgraphmacros}) {
+	<li>This problem uses <tt>PGgraphmacros</tt> a old plotting library. Consider using
+		<a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/graph/plots.html"><tt>Plots.pl</tt></a>
+		or <a target="_blank" href="https://openwebwork.github.io/pg-docs/pod/pg/macros/graph/PGtikz.html"><tt>PGtikz.pl</tt></a></li>
+%}
+% if ($results->{negative}{lines_below_enddocument}) {
+	<li>There is content (code or other text), below the  <tt>ENDDOCUMENT()</tt> line.  Although this
+		is ignored, there shouldn't be content in this area.</li>
+% }
+
+</ul>
+
+
+</div>