@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Improve XHPAST handling of syntax errors
Summary: Currently, a bunch of developers are using #xhpast for writing custom linter rules. As such, we end up with a fair few `XHPASTSyntaxErrorException` in our PHP error logs. I think that throwing an exception is not quite correct in this case because it is somewhat expected that invalid PHP may be entered. Instead, catch the exception and show the user a helpful message.
Test Plan: This doesn't quite work yet... the stream and tree views render as blank but the exceptions still propogate to the error logs. Mostly, I'm not sure how the exception should be rendered for display.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14028
authored by
Joshua Spence
and committed by
Joshua Spence
a2f909f0
c3ecea97
+54
-34
+5
resources/sql/autopatches/20151108.xhpast.stderr.sql
+5
resources/sql/autopatches/20151108.xhpast.stderr.sql
-1
src/applications/phpast/controller/PhabricatorXHPASTViewController.php
-1
src/applications/phpast/controller/PhabricatorXHPASTViewController.php
+1
-1
src/applications/phpast/controller/PhabricatorXHPASTViewFrameController.php
+1
-1
src/applications/phpast/controller/PhabricatorXHPASTViewFrameController.php
+10
-12
src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php
+10
-12
src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php
···
10
10
public function handleRequest(AphrontRequest $request) {
11
11
$id = $request->getURIData('id');
12
12
13
-
$response = new AphrontWebpageResponse();
14
-
$response->setFrameable(true);
15
-
$response->setContent(phutil_tag(
16
-
'frameset',
17
-
array('cols' => '33%, 34%, 33%'),
18
-
array(
19
-
phutil_tag('frame', array('src' => "/xhpast/input/{$id}/")),
20
-
phutil_tag('frame', array('src' => "/xhpast/tree/{$id}/")),
21
-
phutil_tag('frame', array('src' => "/xhpast/stream/{$id}/")),
22
-
)));
23
-
24
-
return $response;
13
+
return id(new AphrontWebpageResponse())
14
+
->setFrameable(true)
15
+
->setContent(phutil_tag(
16
+
'frameset',
17
+
array('cols' => '33%, 34%, 33%'),
18
+
array(
19
+
phutil_tag('frame', array('src' => "/xhpast/input/{$id}/")),
20
+
phutil_tag('frame', array('src' => "/xhpast/tree/{$id}/")),
21
+
phutil_tag('frame', array('src' => "/xhpast/stream/{$id}/")),
22
+
)));
25
23
}
26
24
}
+4
-4
src/applications/phpast/controller/PhabricatorXHPASTViewPanelController.php
+4
-4
src/applications/phpast/controller/PhabricatorXHPASTViewPanelController.php
···
14
14
$this->id = $data['id'];
15
15
$this->storageTree = id(new PhabricatorXHPASTViewParseTree())
16
16
->load($this->id);
17
+
17
18
if (!$this->storageTree) {
18
19
throw new Exception(pht('No such AST!'));
19
20
}
···
65
66
'</html>',
66
67
$content);
67
68
68
-
$response = new AphrontWebpageResponse();
69
-
$response->setFrameable(true);
70
-
$response->setContent($content);
71
-
return $response;
69
+
return id(new AphrontWebpageResponse())
70
+
->setFrameable(true)
71
+
->setContent($content);
72
72
}
73
73
74
74
}
+12
-8
src/applications/phpast/controller/PhabricatorXHPASTViewRunController.php
+12
-8
src/applications/phpast/controller/PhabricatorXHPASTViewRunController.php
···
13
13
$resolved = $future->resolve();
14
14
15
15
// This is just to let it throw exceptions if stuff is broken.
16
-
$parse_tree = XHPASTTree::newFromDataAndResolvedExecFuture(
17
-
$source,
18
-
$resolved);
16
+
try {
17
+
XHPASTTree::newFromDataAndResolvedExecFuture($source, $resolved);
18
+
} catch (XHPASTSyntaxErrorException $ex) {
19
+
// This is possibly expected.
20
+
}
19
21
20
22
list($err, $stdout, $stderr) = $resolved;
21
23
22
-
$storage_tree = new PhabricatorXHPASTViewParseTree();
23
-
$storage_tree->setInput($source);
24
-
$storage_tree->setStdout($stdout);
25
-
$storage_tree->setAuthorPHID($viewer->getPHID());
26
-
$storage_tree->save();
24
+
$storage_tree = id(new PhabricatorXHPASTViewParseTree())
25
+
->setInput($source)
26
+
->setReturnCode($err)
27
+
->setStdout($stdout)
28
+
->setStderr($stderr)
29
+
->setAuthorPHID($viewer->getPHID())
30
+
->save();
27
31
28
32
return id(new AphrontRedirectResponse())
29
33
->setURI('/xhpast/view/'.$storage_tree->getID().'/');
+9
-3
src/applications/phpast/controller/PhabricatorXHPASTViewStreamController.php
+9
-3
src/applications/phpast/controller/PhabricatorXHPASTViewStreamController.php
···
6
6
public function handleRequest(AphrontRequest $request) {
7
7
$storage = $this->getStorageTree();
8
8
$input = $storage->getInput();
9
+
$err = $storage->getReturnCode();
9
10
$stdout = $storage->getStdout();
11
+
$stderr = $storage->getStderr();
10
12
11
-
$tree = XHPASTTree::newFromDataAndResolvedExecFuture(
12
-
$input,
13
-
array(0, $stdout, ''));
13
+
try {
14
+
$tree = XHPASTTree::newFromDataAndResolvedExecFuture(
15
+
$input,
16
+
array($err, $stdout, $stderr));
17
+
} catch (XHPASTSyntaxErrorException $ex) {
18
+
return $this->buildXHPASTViewPanelResponse($ex->getMessage());
19
+
}
14
20
15
21
$tokens = array();
16
22
foreach ($tree->getRawTokenStream() as $id => $token) {
+9
-4
src/applications/phpast/controller/PhabricatorXHPASTViewTreeController.php
+9
-4
src/applications/phpast/controller/PhabricatorXHPASTViewTreeController.php
···
10
10
public function handleRequest(AphrontRequest $request) {
11
11
$storage = $this->getStorageTree();
12
12
$input = $storage->getInput();
13
+
$err = $storage->getReturnCode();
13
14
$stdout = $storage->getStdout();
15
+
$stderr = $storage->getStderr();
14
16
15
-
$tree = XHPASTTree::newFromDataAndResolvedExecFuture(
16
-
$input,
17
-
array(0, $stdout, ''));
17
+
try {
18
+
$tree = XHPASTTree::newFromDataAndResolvedExecFuture(
19
+
$input,
20
+
array($err, $stdout, $stderr));
21
+
} catch (XHPASTSyntaxErrorException $ex) {
22
+
return $this->buildXHPASTViewPanelResponse($ex->getMessage());
23
+
}
18
24
19
25
$tree = phutil_tag('ul', array(), $this->buildTree($tree->getRootNode()));
20
26
return $this->buildXHPASTViewPanelResponse($tree);
21
27
}
22
28
23
29
protected function buildTree($root) {
24
-
25
30
try {
26
31
$name = $root->getTypeName();
27
32
$title = $root->getDescription();
+4
-1
src/applications/phpast/storage/PhabricatorXHPASTViewParseTree.php
+4
-1
src/applications/phpast/storage/PhabricatorXHPASTViewParseTree.php
···
3
3
final class PhabricatorXHPASTViewParseTree extends PhabricatorXHPASTViewDAO {
4
4
5
5
protected $authorPHID;
6
-
7
6
protected $input;
7
+
protected $returnCode;
8
8
protected $stdout;
9
+
protected $stderr;
9
10
10
11
protected function getConfiguration() {
11
12
return array(
12
13
self::CONFIG_COLUMN_SCHEMA => array(
13
14
'authorPHID' => 'phid?',
14
15
'input' => 'text',
16
+
'returnCode' => 'sint32',
15
17
'stdout' => 'text',
18
+
'stderr' => 'text',
16
19
),
17
20
) + parent::getConfiguration();
18
21
}