hhvm / hhast Goto Github PK
View Code? Open in Web Editor NEWMutable AST library for Hack with linting and code migrations
License: MIT License
Mutable AST library for Hack with linting and code migrations
License: MIT License
The NullLiteralToken
always uses null
for the text, even if the source was using a different case. This means that these all get changed when running a migration, even though it's not related to the migration.
This does not happen with true
/false
$ cat test.php
<?hh
NULL;
TRUE;
FALSE;
$ bin/hhast-migrate --optional-shape-fields test.php
$ cat test.php
<?hh
null;
TRUE;
FALSE;
If the path is passed, it will validate everything in that path, ignoring the contents of hhast-lint.json
For example, this will lint vendor/
Large repositories with lots of custom rules aren't going to finish linting in a reasonable amount of time, so LSP mode will need an option to not start a lint run on the entire repository at launch, but rather:
It would be good for users to opt-in to a full suite of linters from a third-party, rather than having to whitelist every specific linter - for example, if a framework decides to ship linters for its' users
This was accidentally permitted in 3.26 with the parser cutover, and we expect to drop support in 3.27
This should be (new Foo())->bar()
It would be best to:
LinterPoweredMigration
base class or similar)Tested on HipHop VM 3.22.0 (rel)
composer require facebook/definition-finder
bin/hhast-migrate --hhvm-3.22-to-3.23 vendor/facebook/definition-finder
Error
Fatal error: Uncaught exception 'Exception' with message 'unexpected JSON kind: extra_token_error' in hhast/codegen/editable_node_from_json.php:371
Stack trace:
#0 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#1 hhast/src/EditableToken.php(145): Facebook\HHAST\EditableNode::fromJSON()
#2 hhast/src/__Private/fold_map.php(24): Closure$Facebook\HHAST\EditableToken::fromJSON()
#3 hhast/src/EditableToken.php(148): Facebook\HHAST\__Private\fold_map()
#4 hhast/codegen/editable_node_from_json.php(21): Facebook\HHAST\EditableToken::fromJSON()
#5 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#6 hhast/codegen/syntax/FunctionCallExpression.php(58): Facebook\HHAST\EditableNode::fromJSON()
#7 hhast/codegen/editable_node_from_json.php(241): Facebook\HHAST\FunctionCallExpression::fromJSON()
#8 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#9 hhast/codegen/syntax/ExpressionStatement.php(34): Facebook\HHAST\EditableNode::fromJSON()
#10 hhast/codegen/editable_node_from_json.php(132): Facebook\HHAST\ExpressionStatement::fromJSON()
#11 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#12 hhast/src/EditableList.php(92): Facebook\HHAST\EditableNode::fromJSON()
#13 hhast/codegen/editable_node_from_json.php(23): Facebook\HHAST\EditableList::fromJSON()
#14 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#15 hhast/codegen/syntax/Script.php(29): Facebook\HHAST\EditableNode::fromJSON()
#16 hhast/codegen/editable_node_from_json.php(47): Facebook\HHAST\Script::fromJSON()
#17 hhast/src/EditableNode.php(129): Facebook\HHAST\__Private\editable_node_from_json()
#18 hhast/src/entrypoints.php(20): Facebook\HHAST\EditableNode::fromJSON()
#19 hhast/src/entrypoints.php(36): Facebook\HHAST\from_json()
#20 hhast/src/__Private/MigrationCLI.php(57): Facebook\HHAST\from_file()
#21 hhast/src/__Private/MigrationCLI.php(74): Facebook\HHAST\__Private\MigrationCLI->migrateFile()
#22 hhast/src/__Private/MigrationCLI.php(98): Facebook\HHAST\__Private\MigrationCLI->migrateDirectory()
#23 hhast/bin/hhast-migrate(18): Facebook\HHAST\__Private\MigrationCLI->mainAsync()
#24 {main}
By default, hhast-migrate --hhvm-3.22-to-3.23
migrates the vendor directory as well.
facebook/definition-finder
is used by hhvm/hhvm-autoload
so a many projects might be affected.
It's somewhat more convenient than actually having to construct a replacement AST for simple fixes/migrations. This should use a manually created EditableNode subclass with no children, and user-specifiable text.
The main downside is that it doesn't stack correctly - if a later migration step needs to modify the same code, it will fail. This should be covered in the documentation.
If there is whitespace at the end of a line, it should be removed.
There's two ways to do this:
Line-based is 'good enough' here, though if you use the AST, you could improve on this by excluding HEREDOCs. Please don't handle HEREDOCs or other edge cases if you do a line-based linter.
Either way, please make sure the CLI output is clear when running the linter, both in your terminal, and non-interactively (e.g. when redirecting to a file with > /tmp/somefile
)
e.g.
"overrides": [
{
"patterns": [ "tests/data/*" ],
"disableAllLinters": true
}
]
This would be a completely new feature; it would make it much easier to debug and work on HHAST-powered tools - replacing workflows such as hh_parse --full-fidelity-json foo.php | jq
This could possibly tie into hhvm/fbmarkdown@2
e.g.:
use type Herp\Derp;
use type Foo\Bar;
use type Foo\Baz;
should become:
use type Herp\Derp;
use type Foo\{
Bar,
Baz,
};
Probably want to publish CodeActions, that send a workplace/applyEdit
Should not be on by default, but should be on when linters = all
Prefer single quoted strings. Exceptions:
"\n"
)Should check:
This option will be supported in HHVM 3.23+; it requires that callsites specify '&' when passing an argument by ref; for example:
$foo = vec[1,3,2];
sort(&$foo);
.hhconfig:
enable_experimental_tc_features=safe_pass_by_ref
With the option off, in 3.23+, the '&' is ignored - but it is an error in master. This can be worked on now if you build HHVM master, or brew tap hhvm/hhvm; brew install hhvm-preview
if working on a mac.
This requires building support for migrations that query and fix typechecker errors, rather than purely operating on the AST
Replace $this->assertEquals()
and similar with expect($a)->tobePHPEqual($b)
In addition to being a pre-requisite to replacing PHPUnit, this solves real issues in tests introduced by Hack array migration: assertEquals()
is always true if both sides are vecs for example, even if the vecs have completely different elements.
This should be a multi-step migration (see https://github.com/hhvm/hhast/blob/master/src/Migrations/ImplicitShapeSubtypesMigration.php for an example). It should:
use function Facebook\FBExpect\expect
if it isn't already present. This should be (in preference order, depending on the file)
use function
if presentuse
declarations if presentnamespace
declaration (or just inside the block) if there is oneThis shouldn't change the base class from PHPUnit.
If windows newlines are present, the linter should suggest that they're replaced with unix newlines.
There's two ways to do this:
Either way, please make sure the CLI output is clear when running the linter, both in your terminal, and non-interactively (e.g. when redirecting to a file with > /tmp/somefile
)
https://github.com/hhvm/hhast/blob/master/src/__Private/CLIBase.php
It's normal to be able to combine multiple short options into a single one - for example, -a -b
should be equivalent to -ab
.
Complications:
-a -b foo
is required - not -abc foo
or -ab foo
-vv
will need to be removed from bin/hhast-lint
; this can be done by changing -v
to increment $verbosity
, instead of setting it to 1For example, the __Override
attribute operates on MethodishDeclaration
s, but should really raise the error against the the decl header, not the entire method.
This matters more now that the LSP/JSON interfaces report ranges.
Probably worth doing given hackfmt --diff
in 3.23 removes them :(
e.g.
use Foo\Bar;
function foo(): Bar {};
... should be autofixed to use type
This should:
https://github.com/hhvm/hhast/blob/master/src/__Private/LinterCLI.php is where you'll need to hook in - but please don't add the diff parsing code there directly. My instinct would be to add a get_filenames_from_diff(string)
function, and use that in LinterCLI
, but I'm open to other approaches.
This would allow updating HHAST independently of HHVM versions
We recommend concatenation (or sprintf) instead; this could be an autofixing linter.
EditableList is always marked as returning EditableNodes. The inference process could refine this.
This is probably not a good first issue as the relationship inference process is extremely slow
This is fairly common...
foo($bar = '123', $baz = '456');
... and has two main problems:
This could be an autofixing-linter, suggesting this instead:
foo(/* bar = */ '123', /* baz = */ '456');
e.g:
$a = await foo();
$b = await bar();
should suggest autofix:
list($a, $b) = await \HH\Asio\va(foo(), bar());
... but this is fine:
$a = await foo();
$b = await bar($a->getFoo());
Some linters have unsafe autofixes, for example, the 'basic assignment' linter also detects accidental invariant($foo = $bar, ...)
Report the file causing them.
For example, right now, running AsyncFunctionAndMethodLinter
against hhvm/definition-finder
throws an exception, but it doesn't say what file causes it
hhast-lint
currently produces user readable output for lint errors. It would be helpful to support a --json
option (similar to hh_client
) to make this parsable by IDEs and other automated tooling.
There are exceptions, but this is a good general rule.
I've been developing with 3.23.2 locally but we've finally rolled out 3.21.3 on the boxes where our linter is run. I've been trying to install both hhast and our repo that includes hhast as a dep, but get the following when trying to install:
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Installation request for facebook/definition-finder v1.6.4 -> satisfiable by facebook/definition-finder[v1.6.4].
- facebook/definition-finder v1.6.4 requires hhvm ~3.23 -> your HHVM version does not satisfy that requirement.
Problem 2
- Installation request for facebook/hack-codegen v3.0.3 -> satisfiable by facebook/hack-codegen[v3.0.3].
- facebook/hack-codegen v3.0.3 requires hhvm >=3.23.0 -> your HHVM version does not satisfy that requirement.
Problem 3
- Installation request for hhvm/hhvm-autoload v1.5.4 -> satisfiable by hhvm/hhvm-autoload[v1.5.4].
- hhvm/hhvm-autoload v1.5.4 requires hhvm ^3.23 -> your HHVM version does not satisfy that requirement.
Problem 4
- Installation request for hhvm/type-assert v3.2.0 -> satisfiable by hhvm/type-assert[v3.2.0].
- hhvm/type-assert v3.2.0 requires hhvm ^3.23.0 -> your HHVM version does not satisfy that requirement.
Problem 5
- Installation request for 91carriage/phpunit-hhi 5.7.2 -> satisfiable by 91carriage/phpunit-hhi[5.7.2].
- 91carriage/phpunit-hhi 5.7.2 requires hhvm >=3.23.0 -> your HHVM version does not satisfy that requirement.
Problem 6
- hhvm/hhvm-autoload v1.5.4 requires hhvm ^3.23 -> your HHVM version does not satisfy that requirement.
- hhvm/hsl v1.2.0 requires hhvm/hhvm-autoload ^1.4 -> satisfiable by hhvm/hhvm-autoload[v1.5.4].
- Installation request for hhvm/hsl v1.2.0 -> satisfiable by hhvm/hsl[v1.2.0].
I know 3.21.3 probably isn't fun to support, so I've tried to fork and figure out a set of deps that does work up until v0.5 and got this far:
{
"name": "hhvm/hhast",
"bin": [ "bin/hhast-lint" ],
"require-dev": {
"facebook/hack-codegen": "=3.0.1",
"phpunit/phpunit": "^5.7",
"91carriage/phpunit-hhi": "=5.7.1",
"facebook/fbexpect": "^0.3.0",
"hhvm/hhvm-autoload": "=1.5.3",
"facebook/definition-finder": "=1.6.3"
},
"require": {
"hhvm": "^3.21.0",
"hhvm/hsl": "=1.1.0",
"hhvm/type-assert": "=3.1.0"
}
}
but when I try to get hhast to lint itself:
bradley@slack:~/hhast$ hhvm bin/hhast-lint .
Warning: Destructor threw an object exception: exception 'HH\InvariantException' with message 'counter Facebook\HHAST\__Private\LinterCLI destroyed without calling ::end()' in /home/bradley/hhast/src/__Private/PerfCounter.php:51
Stack trace:
#0 /home/bradley/hhast/src/__Private/PerfCounter.php(51): HH\invariant_violation()
#1 /home/bradley/hhast/src/__Private/LinterCLI.php(155): Facebook\HHAST\__Private\PerfCounter->__destruct()
#2 /home/bradley/hhast/bin/hhast-lint(38): Facebook\HHAST\__Private\LinterCLI::mainAsync()
#3 {main} in /home/bradley/hhast/src/__Private/LinterCLI.php on line 155
Fatal error: Uncaught exception 'Exception' with message 'Invalid configuration file: /home/bradley/hhast/hhast-lint.json
Expected type 'vec<T>', got type 'array'
Type trace:
#0 shape[roots]
' in /home/bradley/hhast/src/__Private/LinterCLIConfig.php:243
Stack trace:
#0 /home/bradley/hhast/src/__Private/LinterCLIConfig.php(97): Facebook\HHAST\__Private\LinterCLIConfig::getConfigFromFile()
#1 /home/bradley/hhast/src/__Private/LinterCLIConfig.php(98): Facebook\HHAST\__Private\LinterCLIConfig::getFromConfigFile$memoize_impl()
#2 /home/bradley/hhast/src/__Private/LinterCLIConfig.php(121): Facebook\HHAST\__Private\LinterCLIConfig::getFromConfigFile()
#3 /home/bradley/hhast/src/__Private/LinterCLIConfig.php(127): Facebook\HHAST\__Private\LinterCLIConfig::getForPathImpl$memoize_impl()
#4 /home/bradley/hhast/src/__Private/LinterCLIConfig.php(109): Facebook\HHAST\__Private\LinterCLIConfig::getForPathImpl()
#5 /home/bradley/hhast/src/__Private/LinterCLI.php(138): Facebook\HHAST\__Private\LinterCLIConfig::getForPath()
#6 /home/bradley/hhast/bin/hhast-lint(38): Facebook\HHAST\__Private\LinterCLI->mainAsync()
#7 {main}
at this point I'm not sure what else to try. Any suggestions?
This is a more opinionated one: it hurts greppability and readability.
3.24's schema contains backwards-incompatible changes. We need to either:
We probably also should build support for verifying that hh_parse --schema
returns a recognized schema version string.
There may still be changes - current diff from 3.23 is:
https://gist.github.com/fredemmott/85632d66bc09413e00ebf8710e5fa0af
Some highlights:
- { "field_name" : "visibility" },
+ { "field_name" : "modifiers" },
- { "kind_name" : "XHPAttribute",
...
+ { "kind_name" : "XHPSimpleAttribute",
...
+ { "kind_name" : "XHPSpreadAttribute",
- { "field_name" : "parameter_types" },
+ { "field_name" : "parameter_list" },
These currently execute hh_client for every file - we only need to do it once per codebase, and we don't need to traverse the file tree at all.
Promoted constructor arguments should be lowerCamelCase
For example, if vendor/hh_autoload.php
is changed, it shouldn't be linted
With the HSL, we're establishing a new convention for async functions (there's currently none):
_async
or _asyncx
Async
or Asyncx
This should be an AST-based linter, but should not support autofixing (as all the callsites would need updating too). It should use https://github.com/hhvm/hhast/blob/master/src/Linters/FunctionNamingLinterTrait.php -
https://github.com/hhvm/hhast/blob/master/src/Linters/CamelCasedMethodsUnderscoredFunctionsLinter.php may be a useful reference for this.
Solve issues such as: facebookarchive/fbctf#576
It should take a fully qualified function/method name (e.g. foo()
, Foo\bar
, Foo::bar()
, Foo\Bar::baz()
) as input. It should then:
\HH\Asio\join()
with await in simple cases. Replace with easily-greppable wrapper (maybe add \HHAST\PartiallyMigrated_FIXME\Asio\join()
?This is less of an "issue" and more of a question:
Is there a good way to stub or mock the shouldLintFile
of linters in tests?
The canonical linters are all marked final
, and even if they weren't PHPUnit's createMock()
appears to not handle Hack which is understandable. Any advice would be greatly appreciated, as we have linters that only run on files whose names match regexs but would like to be able to have test coverage. Thanks!
This should ban:
$x = [];
$y = array();
etc
Autofixable: fixer could use hh_client --search --json
to find out if use type
is appropriate; if not, fall back to use namespace
. Not 100% accurate, but probably close enough.
return new self($a != $b);
This raises a NoPHPEquality lint error - but I can't find any place to insert a HHAST_IGNORE_ERROR that suppresses it.
for example, if you have HHAST's tree open in your IDE, and run the tests, the IDE starts showing lint errors for the .out files
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.