From 84c0ea3790a39e6719986095f05f2a09f15e8764 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 6 Mar 2022 07:35:46 +0100 Subject: [PATCH] Treat unknown prototype props as unknown (#4428) * Treat unknown prototype methods as unknown * Improve coverage --- src/ast/nodes/shared/ObjectPrototype.ts | 39 ++++++++++++++++++- .../watch-config-initial-error/_config.js | 3 +- .../_expected.js | 4 +- .../main.js | 8 ++-- .../samples/class-method-access/_expected.js | 2 +- test/form/samples/class-method-access/main.js | 6 +-- .../minimal-this-mutation/_expected.js | 6 --- .../samples/minimal-this-mutation/main.js | 7 ---- .../_config.js | 6 +++ .../_expected.js | 3 ++ .../main.js | 3 ++ .../_config.js | 6 +++ .../_expected.js | 5 +++ .../main.js | 5 +++ .../samples/optional-chaining/_expected.js | 2 +- test/form/samples/optional-chaining/main.js | 4 +- .../_expected.js | 7 +++- .../super-class-prototype-values/_expected.js | 2 - .../super-class-prototype-values/main.js | 4 -- .../samples/undefined-properties/_config.js | 3 -- .../samples/undefined-properties/_expected.js | 10 ----- .../form/samples/undefined-properties/main.js | 11 ------ .../_config.js | 3 ++ .../retain-unknown-prototype-access/main.js | 10 +++++ 24 files changed, 100 insertions(+), 59 deletions(-) create mode 100644 test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_config.js create mode 100644 test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_expected.js create mode 100644 test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/main.js create mode 100644 test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_config.js create mode 100644 test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_expected.js create mode 100644 test/form/samples/object-expression/nested-literal-value-without-access-side-effect/main.js delete mode 100644 test/form/samples/undefined-properties/_config.js delete mode 100644 test/form/samples/undefined-properties/_expected.js delete mode 100644 test/form/samples/undefined-properties/main.js create mode 100644 test/function/samples/retain-unknown-prototype-access/_config.js create mode 100644 test/function/samples/retain-unknown-prototype-access/main.js diff --git a/src/ast/nodes/shared/ObjectPrototype.ts b/src/ast/nodes/shared/ObjectPrototype.ts index e4d217875dd..ca2b8112f61 100644 --- a/src/ast/nodes/shared/ObjectPrototype.ts +++ b/src/ast/nodes/shared/ObjectPrototype.ts @@ -1,3 +1,6 @@ +import { EVENT_CALLED, NodeEvent } from '../../NodeEvents'; +import { ObjectPath, ObjectPathKey, UNKNOWN_PATH } from '../../utils/PathTracker'; +import { ExpressionEntity, LiteralValueOrUnknown, UnknownValue } from './Expression'; import { METHOD_RETURNS_BOOLEAN, METHOD_RETURNS_STRING, @@ -5,6 +8,40 @@ import { } from './MethodTypes'; import { ObjectEntity, type PropertyMap } from './ObjectEntity'; +const isInteger = (prop: ObjectPathKey): boolean => typeof prop === 'string' && /^\d+$/.test(prop); + +// This makes sure unknown properties are not handled as "undefined" but as +// "unknown" but without access side effects. An exception is done for numeric +// properties as we do not expect new builtin properties to be numbers, this +// will improve tree-shaking for out-of-bounds array properties +const OBJECT_PROTOTYPE_FALLBACK: ExpressionEntity = + new (class ObjectPrototypeFallbackExpression extends ExpressionEntity { + deoptimizeThisOnEventAtPath( + event: NodeEvent, + path: ObjectPath, + thisParameter: ExpressionEntity + ): void { + if (event === EVENT_CALLED && path.length === 1 && !isInteger(path[0])) { + thisParameter.deoptimizePath(UNKNOWN_PATH); + } + } + + getLiteralValueAtPath(path: ObjectPath): LiteralValueOrUnknown { + // We ignore number properties as we do not expect new properties to be + // numbers and also want to keep handling out-of-bound array elements as + // "undefined" + return path.length === 1 && isInteger(path[0]) ? undefined : UnknownValue; + } + + hasEffectsWhenAccessedAtPath(path: ObjectPath): boolean { + return path.length > 1; + } + + hasEffectsWhenAssignedAtPath(path: ObjectPath): boolean { + return path.length > 1; + } + })(); + export const OBJECT_PROTOTYPE = new ObjectEntity( { __proto__: null, @@ -15,6 +52,6 @@ export const OBJECT_PROTOTYPE = new ObjectEntity( toString: METHOD_RETURNS_STRING, valueOf: METHOD_RETURNS_UNKNOWN } as unknown as PropertyMap, - null, + OBJECT_PROTOTYPE_FALLBACK, true ); diff --git a/test/cli/samples/watch/watch-config-initial-error/_config.js b/test/cli/samples/watch/watch-config-initial-error/_config.js index 7faa10b357a..4b6cd1c093c 100644 --- a/test/cli/samples/watch/watch-config-initial-error/_config.js +++ b/test/cli/samples/watch/watch-config-initial-error/_config.js @@ -12,7 +12,8 @@ module.exports = { writeFileSync(configFile, 'throw new Error("Config contains initial errors");'); }, after() { - unlinkSync(configFile); + // synchronous sometimes does not seem to work, probably because the watch is not yet removed properly + setTimeout(() => unlinkSync(configFile), 300); }, async abortOnStderr(data) { if (data.includes('Config contains initial errors')) { diff --git a/test/form/samples/avoid-unnecessary-conditional-deopt/_expected.js b/test/form/samples/avoid-unnecessary-conditional-deopt/_expected.js index 92a9ac36f0f..8274a7e0e1a 100644 --- a/test/form/samples/avoid-unnecessary-conditional-deopt/_expected.js +++ b/test/form/samples/avoid-unnecessary-conditional-deopt/_expected.js @@ -2,7 +2,7 @@ console.log('not modified'); let x2 = false; modifyX2(); -const obj2 = {}; +const obj2 = { modified: false }; (x2 ? obj2 : {}).modified = true; if (obj2.modified) console.log('modified'); @@ -16,7 +16,7 @@ console.log('not modified'); let x4 = false; modifyX4(); -const obj4 = {}; +const obj4 = { modified: false }; (x4 && obj4).modified = true; if (obj4.modified) console.log('modified'); diff --git a/test/form/samples/avoid-unnecessary-conditional-deopt/main.js b/test/form/samples/avoid-unnecessary-conditional-deopt/main.js index c00050cbaf4..4c7337a5824 100644 --- a/test/form/samples/avoid-unnecessary-conditional-deopt/main.js +++ b/test/form/samples/avoid-unnecessary-conditional-deopt/main.js @@ -1,6 +1,6 @@ let x1 = false; modifyX1(); -const obj1 = {}; +const obj1 = { modified: false }; x1 ? obj1 : {}; if (obj1.modified) console.log('should not happen'); @@ -12,7 +12,7 @@ function modifyX1() { let x2 = false; modifyX2(); -const obj2 = {}; +const obj2 = { modified: false }; (x2 ? obj2 : {}).modified = true; if (obj2.modified) console.log('modified'); @@ -24,7 +24,7 @@ function modifyX2() { let x3 = false; modifyX3(); -const obj3 = {}; +const obj3 = { modified: false }; x3 && obj3; if (obj3.modified) console.log('should not happen'); @@ -36,7 +36,7 @@ function modifyX3() { let x4 = false; modifyX4(); -const obj4 = {}; +const obj4 = { modified: false }; (x4 && obj4).modified = true; if (obj4.modified) console.log('modified'); diff --git a/test/form/samples/class-method-access/_expected.js b/test/form/samples/class-method-access/_expected.js index 034e5c35fe7..6bebedb86d1 100644 --- a/test/form/samples/class-method-access/_expected.js +++ b/test/form/samples/class-method-access/_expected.js @@ -3,7 +3,7 @@ console.log('retained'); class Used { static method() {} static get getter() { - return { foo: {} }; + return { foo: { throws: null }, throws: null }; } } Used.method.doesNotExist.throws; diff --git a/test/form/samples/class-method-access/main.js b/test/form/samples/class-method-access/main.js index 25e980dc120..f017bee050d 100644 --- a/test/form/samples/class-method-access/main.js +++ b/test/form/samples/class-method-access/main.js @@ -10,7 +10,7 @@ else console.log('removed'); class Used { static method() {} static get getter() { - return { foo: {} }; + return { foo: { throws: null }, throws: null }; } } @@ -30,5 +30,5 @@ Used.method.reassigned = 1; Used.getter.reassigned = 2; class ValueEffect { - static foo -} \ No newline at end of file + static foo; +} diff --git a/test/form/samples/minimal-this-mutation/_expected.js b/test/form/samples/minimal-this-mutation/_expected.js index bf850340e03..58798f2302d 100644 --- a/test/form/samples/minimal-this-mutation/_expected.js +++ b/test/form/samples/minimal-this-mutation/_expected.js @@ -40,9 +40,3 @@ const obj5 = { obj5.mutateNestedProp(); if (obj5.nested.prop) console.log('unimportant'); else console.log('retained'); - -const obj6 = { - prop: true -}; -obj6.doesNotExist(); -console.log('retained'); diff --git a/test/form/samples/minimal-this-mutation/main.js b/test/form/samples/minimal-this-mutation/main.js index a70c3d521e5..39643af9776 100644 --- a/test/form/samples/minimal-this-mutation/main.js +++ b/test/form/samples/minimal-this-mutation/main.js @@ -47,10 +47,3 @@ const obj5 = { obj5.mutateNestedProp(); if (obj5.nested.prop) console.log('unimportant'); else console.log('retained'); - -const obj6 = { - prop: true -}; -obj6.doesNotExist(); -if (obj6.prop) console.log('retained'); -else console.log('removed'); diff --git a/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_config.js b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_config.js new file mode 100644 index 00000000000..176ab160d77 --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'treats mutating nested properties as side effects', + options: { + treeshake: { propertyReadSideEffects: false } + } +}; diff --git a/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_expected.js b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_expected.js new file mode 100644 index 00000000000..a1aec95dee4 --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/_expected.js @@ -0,0 +1,3 @@ +const obj = { __proto__: null }; + +obj.foo.bar = 'retained'; diff --git a/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/main.js b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/main.js new file mode 100644 index 00000000000..a1aec95dee4 --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-assignment-without-access-side-effect/main.js @@ -0,0 +1,3 @@ +const obj = { __proto__: null }; + +obj.foo.bar = 'retained'; diff --git a/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_config.js b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_config.js new file mode 100644 index 00000000000..fdda6575331 --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'uses an unknown value for nested properties', + options: { + treeshake: { propertyReadSideEffects: false } + } +}; diff --git a/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_expected.js b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_expected.js new file mode 100644 index 00000000000..b80a0b6731d --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/_expected.js @@ -0,0 +1,5 @@ +const obj = { __proto__: null }; + +if (obj.foo.bar) { + console.log('retained'); +} diff --git a/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/main.js b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/main.js new file mode 100644 index 00000000000..b80a0b6731d --- /dev/null +++ b/test/form/samples/object-expression/nested-literal-value-without-access-side-effect/main.js @@ -0,0 +1,5 @@ +const obj = { __proto__: null }; + +if (obj.foo.bar) { + console.log('retained'); +} diff --git a/test/form/samples/optional-chaining/_expected.js b/test/form/samples/optional-chaining/_expected.js index 3f65cf02583..8389fd5e931 100644 --- a/test/form/samples/optional-chaining/_expected.js +++ b/test/form/samples/optional-chaining/_expected.js @@ -1,3 +1,3 @@ -const obj = {}; +const obj = { __proto__: null }; obj.foo?.bar; obj.foo?.(); diff --git a/test/form/samples/optional-chaining/main.js b/test/form/samples/optional-chaining/main.js index 4f6aab57e58..c4cdb5adf2f 100644 --- a/test/form/samples/optional-chaining/main.js +++ b/test/form/samples/optional-chaining/main.js @@ -1,4 +1,4 @@ -const obj = {}; -obj?.foo +const obj = { __proto__: null }; +obj?.foo; obj.foo?.bar; obj.foo?.(); diff --git a/test/form/samples/side-effects-class-getters-setters/_expected.js b/test/form/samples/side-effects-class-getters-setters/_expected.js index c09724d4c6a..997f18debfa 100644 --- a/test/form/samples/side-effects-class-getters-setters/_expected.js +++ b/test/form/samples/side-effects-class-getters-setters/_expected.js @@ -23,7 +23,12 @@ class RetainedSuper { } class RetainedSub extends RetainedSuper {} RetainedSub.a; -log(); + +// class fields are not part of the prototype +class RemovedProtoValue { + a = true; +} +if (!RemovedProtoValue.prototype.a) log(); class DeoptProto { a = true; diff --git a/test/form/samples/super-classes/super-class-prototype-values/_expected.js b/test/form/samples/super-classes/super-class-prototype-values/_expected.js index 09f0c0d87ce..60cdfd88e49 100644 --- a/test/form/samples/super-classes/super-class-prototype-values/_expected.js +++ b/test/form/samples/super-classes/super-class-prototype-values/_expected.js @@ -1,7 +1,5 @@ console.log('retained'); console.log('retained'); -// Note that isTrueProp is not part of the prototype -console.log('retained'); const prop = { isTrue: true }; class SuperDeopt { diff --git a/test/form/samples/super-classes/super-class-prototype-values/main.js b/test/form/samples/super-classes/super-class-prototype-values/main.js index 0fb92a2f737..1fce6f66d3d 100644 --- a/test/form/samples/super-classes/super-class-prototype-values/main.js +++ b/test/form/samples/super-classes/super-class-prototype-values/main.js @@ -1,5 +1,4 @@ class SuperValues { - isTrueProp = true; get isTrue() { return true; } @@ -12,9 +11,6 @@ if (Values.prototype.isTrue) console.log('retained'); else console.log('removed'); if (Values.prototype.prop.isTrue) console.log('retained'); else console.log('removed'); -// Note that isTrueProp is not part of the prototype -if (Values.prototype.isTrueProp) console.log('removed'); -else console.log('retained'); const prop = { isTrue: true }; class SuperDeopt { diff --git a/test/form/samples/undefined-properties/_config.js b/test/form/samples/undefined-properties/_config.js deleted file mode 100644 index 29b3cc673cb..00000000000 --- a/test/form/samples/undefined-properties/_config.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - description: 'detects undefined properties' -}; diff --git a/test/form/samples/undefined-properties/_expected.js b/test/form/samples/undefined-properties/_expected.js deleted file mode 100644 index f97408e6838..00000000000 --- a/test/form/samples/undefined-properties/_expected.js +++ /dev/null @@ -1,10 +0,0 @@ -var a = { - b: { - c: 'd' - } -}; - -console.log('retained'); - -if (a.c.d) console.log('retained for effect'); -else console.log('retained for effect'); diff --git a/test/form/samples/undefined-properties/main.js b/test/form/samples/undefined-properties/main.js deleted file mode 100644 index bf2ad33721a..00000000000 --- a/test/form/samples/undefined-properties/main.js +++ /dev/null @@ -1,11 +0,0 @@ -var a = { - b: { - c: 'd' - } -}; - -if (a.b.d) console.log('removed'); -else console.log('retained'); - -if (a.c.d) console.log('retained for effect'); -else console.log('retained for effect'); \ No newline at end of file diff --git a/test/function/samples/retain-unknown-prototype-access/_config.js b/test/function/samples/retain-unknown-prototype-access/_config.js new file mode 100644 index 00000000000..576db2d0c73 --- /dev/null +++ b/test/function/samples/retain-unknown-prototype-access/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'treats unknown builtin prototype properties as unknown' +}; diff --git a/test/function/samples/retain-unknown-prototype-access/main.js b/test/function/samples/retain-unknown-prototype-access/main.js new file mode 100644 index 00000000000..c7c27ed245f --- /dev/null +++ b/test/function/samples/retain-unknown-prototype-access/main.js @@ -0,0 +1,10 @@ +Object.defineProperty(Array.prototype, 'isEmpty', { + get() { + return this.length === 0; + } +}); + +const array = []; +assert.strictEqual(array.isEmpty ? 'works' : 'broken', 'works'); +array.push('foo'); +assert.strictEqual(array.isEmpty ? 'broken' : 'works', 'works');