-
Notifications
You must be signed in to change notification settings - Fork 43
Fix lint errors emitted by HHClientLinter #406
Conversation
The hh_client --lint linter catches far more cases. Interface override, override from traits are those I took note of. The codegen directory was intentionally ignored in this PR.
Required some special casing in CodegenSyntax.
Codegen lint errors are intentionally ignored in this commit. This commit will fail CI because of that.
I confirmed that this is correct by doing codegenning this first. $check = (Node $node) ==> $node; $check($this->_the_thing); return true; This did not generate type errors for passing a nullable for Node.
| This will become problematic when reference (object) typed properties are added. | ||
| All properties are primitive, structural, or value arrays as of November 2021. | ||
| Calling the constructor explcitly would cost a lot of needless re-`Vec\sort_by()` work. | ||
| HHAST_FIXME[5562] */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we let schema be a memoized function to avoid recalculation of Vec\sort_by
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schema's rather large, and memoize is wasteful
The more common way to address this is to make the constructor protected, and move the expensive computation that's currently in the constructor to a public static method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently withoutHackfmt is called immediately after a CodegenBase is created, therefore the constructor will be called twice if we remove the clone operator. Memoization would not cost more memory than the current implementation because anyway schema would be stored on CodegenBase. The problem the that memoization could solve is to avoid the schema calculation in the first call to the constructor, because the fist instance can be discarded before the memoized function getting executed.
|
This is going to need cleanup once #408 is resolved |
Intentionally skipped three, see hhast-lint.json