JavaScript: Restrict InstanceFieldAsPropWrite to fields with initializers. - #920
Merged
Merged
Conversation
Contributor
|
LGTM |
Author
|
Performance is unaffected (internal link). As expected, a large number of results go away; the ones I checked are false positives of the kind we saw on LGTM.com. |
asger-semmle
approved these changes
Feb 12, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As an alternative to #918, we could consider this (partial) revert of 60cef60 (which was added to the PR at my unwise suggestion).
Both in Flow and in TypeScript, a field declaration does not give rise to any actual JavaScript code (in particular, the corresponding property is not initialised to
undefinedor anything), so there is no reason to model such declarations asPropWrites.I have verified that this fixes all five dead-store-of-property alerts on
g/wabilin/MumiGivePandg/ionjs-dev/ionjs(the projects we've had reports about on discuss.lgtm.com). A full evaluation is ongoing, but performance looks unaffected so far.