Skip to content

Commit

Permalink
Random bug fixes (hhvm#542)
Browse files Browse the repository at this point in the history
* Document bug comparison empty statements
The operators `<=` and `>=` should be treated as empty.
They are currently treated as assignments.

* Fix false negative, `<=` and `>=` in NoEmptyStatementsLinter

* Document bug NodeList::getChildrenOfItemsOfType() does not refine

* Fix type refinement of NodeList::getChildrenOfItemsOfType<T>()

* Add NodeList::getChildrenOfItemsByType<T>()
This completes this list:
 - Node->getChildrenOfType<T>()
 - Node->getDescendantsOfType<T>()
 - Node->getFirstDescendantOfType<T>()
 - NodeList->getChildrenOfItemsByType<T>()

* Remove reference in comment to createMaybeEmptyList
This function does not exist anymore.

* Post commit formatting

* Lint clean

* Lint clean (with HHClientLinter)

* Post commit formatting

* One (last) lint error from HHClientLinter

* Demonstrate the `() ==> constant` can not be parsed

* Regenerate codegen, allowing constants as lambda bodies
The Package* classes were deleted by codegen.
I have ignored these deletions for backwards compat.

* Limit semaphore to cpu count
I don't got no 32 hardware threads

* Fix lint error in syntax example
  • Loading branch information
lexidor authored Oct 31, 2023
1 parent d2345af commit 2339345
Show file tree
Hide file tree
Showing 17 changed files with 198 additions and 44 deletions.
5 changes: 4 additions & 1 deletion codegen/inferred_relationships.hack

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions codegen/syntax/LambdaExpression.hack

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions codegen/syntax/QualifiedName.hack

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions codegen/tokens/NameToken.hack

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions src/Linters/NoEmptyStatementsLinter.hack
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ final class NoEmptyStatementsLinter extends AutoFixingASTLinter {
/**
* Returns whether the given token is an assignment operator.
*
* This list is all the types returned from ExpressionStatement::getOperator
* This list is all the types returned from BinaryExpression::getOperator
* that include "Equal" and are not comparison operators (==, >=, etc.);
*/
private function isAssignmentOperator(Token $op): bool {
Expand All @@ -140,9 +140,7 @@ final class NoEmptyStatementsLinter extends AutoFixingASTLinter {
$op is CaratEqualToken ||
$op is DotEqualToken ||
$op is EqualToken ||
$op is GreaterThanEqualToken ||
$op is GreaterThanGreaterThanEqualToken ||
$op is LessThanEqualToken ||
$op is LessThanLessThanEqualToken ||
$op is MinusEqualToken ||
$op is PercentEqualToken ||
Expand Down
4 changes: 2 additions & 2 deletions src/Linters/UnusedUseClauseLinter.hack
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ final class UnusedUseClauseLinter extends AutoFixingASTLinter {
$as = $name->getText();
} else {
invariant($name is QualifiedName, 'Unhandled name type');
$as = $name->getParts()->getChildrenOfItemsOfType(NameToken::class)
|> (C\lastx($$) as nonnull)->getText();
$as = $name->getParts()->getChildrenOfItemsByType<NameToken>()
|> C\lastx($$)->getText();
}
}
if ($kind is NamespaceToken) {
Expand Down
15 changes: 5 additions & 10 deletions src/Linters/UseStatementWIthoutKindLinter.hack
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,8 @@ final class UseStatementWithoutKindLinter extends AutoFixingASTLinter {
}
$name = $clause->getName();
if ($name is QualifiedName) {
return (
C\lastx(
$name->getParts()->getChildrenOfItemsOfType(NameToken::class),
) as nonnull
return C\lastx(
$name->getParts()->getChildrenOfItemsByType<NameToken>(),
)->getText();
}
invariant(
Expand All @@ -73,10 +71,8 @@ final class UseStatementWithoutKindLinter extends AutoFixingASTLinter {
// We need to look at the full file to figure out if this should be a
// `use type`, or `use namespace`
$used = $this->getUnresolvedReferencedNames();
$used_as_ns = C\any(
$names,
$name ==> C\contains($used['namespaces'], $name),
);
$used_as_ns =
C\any($names, $name ==> C\contains($used['namespaces'], $name));
$used_as_type = C\any($names, $name ==> C\contains($used['types'], $name));

$leading = $node->getClauses()->getFirstTokenx()->getLeadingWhitespace();
Expand All @@ -94,8 +90,7 @@ final class UseStatementWithoutKindLinter extends AutoFixingASTLinter {
}

<<__Memoize>>
private function getUnresolvedReferencedNames(
): shape(
private function getUnresolvedReferencedNames(): shape(
'namespaces' => keyset<string>,
'types' => keyset<string>,
'functions' => keyset<string>,
Expand Down
6 changes: 3 additions & 3 deletions src/Migrations/HSLMigration.hack
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ final class HSLMigration extends BaseMigration {
}
$found_prefix = true;
foreach ($parts as $i => $token) {
if ($token?->getText() === $search[$i]) {
if ($token->getText() === $search[$i]) {
continue;
}
$found_prefix = false;
Expand Down Expand Up @@ -569,14 +569,14 @@ final class HSLMigration extends BaseMigration {
}

foreach ($parts as $i => $token) {
if ($i < 2 && $token?->getText() !== $search[$i]) {
if ($i < 2 && $token->getText() !== $search[$i]) {
break;
}

if ($i === 2) {
// we found an HH\Lib\* use statement, add the node and suffix
$nodes[] = $decl;
$ns = HslNamespace::coerce($token?->getText());
$ns = HslNamespace::coerce($token->getText());
if ($ns !== null) {
$suffixes[] = $ns;
}
Expand Down
5 changes: 3 additions & 2 deletions src/__Private/codegen/CodegenBase.hack
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ abstract class CodegenBase {
HHAST\ILambdaBody::class => keyset[
HHAST\IExpression::class,
HHAST\CompoundStatement::class,
// Constants are not wrapped in a name expression on the RHS of `==>`.
HHAST\NameToken::class,
HHAST\QualifiedName::class,
],
HHAST\ILambdaSignature::class => keyset[
HHAST\VariableExpression::class,
Expand All @@ -239,8 +242,6 @@ abstract class CodegenBase {
HHAST\ParameterDeclaration::class,
HHAST\PropertyDeclaration::class,
HHAST\LambdaExpression::class,
// HHAST\Php7AnonymousFunction::class : not valid in hack. No attributes
// if not hack
],
HHAST\INameishNode::class => keyset[
HHAST\NameToken::class,
Expand Down
2 changes: 1 addition & 1 deletion src/__Private/codegen/CodegenRelations.hack
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ final class CodegenRelations extends CodegenBase {

$relationships = new Ref(dict[]);
$queue = new Async\Semaphore(
/* limit = */ 32,
\cpu_get_count(),
async $file ==> {
try {
$links = await $this->getRelationsInFileAsync($file);
Expand Down
15 changes: 15 additions & 0 deletions src/__Private/codegen/data/LambdaBody.SyntaxExample.hack
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/
namespace Facebook\HHAST\__Private\SyntaxExamples;

function lambda_body(): void {
$_ = () ==> Qualified\CONSTANT;
$_ = () ==> CONSTANT;
$_ = () ==> 1 + CONSTANT;
}
50 changes: 37 additions & 13 deletions src/nodes/NodeList.hack
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,23 @@
namespace Facebook\HHAST;

use namespace HH\Lib\{C, Str, Vec};
use type Facebook\HHAST\_Private\SoftDeprecated;

/* HHAST_IGNORE_ALL[5624] */
final class NodeList<+Titem as Node> extends Node {
const string SYNTAX_KIND = 'list';
/**
* Use `NodeList::createMaybeEmptyList()` or
* `NodeList::createNonEmptyListNull()` instead to be explicit
* about desired behavior.
* Use `NodeList::createMaybeEmptyList(vec[])` or `null` instead of
* `new NodeList(vec[])` if you know the vec is always empty.
* A parsed Hack AST doesn't contain empty NodeLists.
*
* Side note: Places where you'd expect to find an empty NodeList:
* ```
* function no_params( ): void {}
* // ^
* ```
* The Hack parser places a "missing" node at the carat.
* HHAST uses `null` to represent them.
*/
<<__Override>>
public function __construct(
Expand All @@ -42,17 +51,33 @@ final class NodeList<+Titem as Node> extends Node {
return $this->_children;
}

public function getChildrenOfItems<T as ?Node>(
): vec<T> where Titem as ListItem<T> {
public function getChildrenOfItems<T as ?Node>(): vec<T>
where
Titem as ListItem<T> {
return Vec\map($this->getChildren(), $child ==> $child->getItem());
}

<<SoftDeprecated('$node->getChildrenOfItemsByType<T>()')>>
public function getChildrenOfItemsOfType<T as ?Node>(
classname<T> $what,
): vec<T> where Titem as ListItem<T> {
): vec<T> where Titem as ListItem<?Node> {
$out = vec[];
foreach ($this->getChildrenOfItems() as $item) {
if (\is_a($item, $what)) {
$out[] = \HH\FIXME\UNSAFE_CAST<?Node, T>(
$item,
'is_a($item, $what) ~= $item is T',
);
}
}
return $out;
}

public function getChildrenOfItemsByType<<<__Enforceable>> reify T as Node>(
): vec<T> where Titem as ListItem<?Node> {
$out = vec[];
foreach ($this->getChildrenOfItems() as $item) {
if ($item is T) {
$out[] = $item;
}
}
Expand Down Expand Up @@ -116,7 +141,7 @@ final class NodeList<+Titem as Node> extends Node {
'source' => $source,
'offset' => $offset,
'width' => $current_position - $offset,
)
),
);
}

Expand Down Expand Up @@ -180,9 +205,8 @@ final class NodeList<+Titem as Node> extends Node {
if (!C\contains($this->_children, $old)) {
return $this;
}
return new NodeList(
Vec\map($this->_children, $c ==> $c === $old ? $new : $c),
);
return
new NodeList(Vec\map($this->_children, $c ==> $c === $old ? $new : $c));
}

public function insertBefore<Tchild super Titem as Node>(
Expand Down Expand Up @@ -228,9 +252,9 @@ final class NodeList<+Titem as Node> extends Node {
return new NodeList($new);
}

public function withoutItemWithChild<Tinner as Node>(
Tinner $inner,
): this where Titem as ListItem<Tinner> {
public function withoutItemWithChild<Tinner as Node>(Tinner $inner): this
where
Titem as ListItem<Tinner> {
$new = Vec\filter($this->_children, $c ==> $c->getItem() !== $inner);
if ($new === $this->_children) {
return $this;
Expand Down
29 changes: 29 additions & 0 deletions tests/NoParamsIsAMissingNodeTest.hack
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

namespace Facebook\HHAST;

use type Facebook\HackTest\HackTest;
use function Facebook\FBExpect\expect;

final class NoParamsIsAMissingNodeTest extends HackTest {
/**
* @see `NodeList::__construct()`
* If this test fails, don't try and fix it.
* Just remove the comment (and this test) if this ever starts failing.
*/
public function testTheFollowingCommentStaysCorrect(): void {
$_error = null;
$json = \HH\ffp_parse_string('function no_params( ): void {}')
|> \json_encode_pure($$, inout $_error);
expect($json)->toContainSubstring(
'"function_parameter_list":{"kind":"missing"}',
);
}
}
Loading

0 comments on commit 2339345

Please sign in to comment.