From ee8cd72af47a5c7322e28d0f36d577950e8d4b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 27 Oct 2021 13:19:31 +0000 Subject: [PATCH 1/4] Add tests checking return type of StringStream transforms --- test/streams/string/creation.spec.ts | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/streams/string/creation.spec.ts b/test/streams/string/creation.spec.ts index ee76047..5d2a9ad 100644 --- a/test/streams/string/creation.spec.ts +++ b/test/streams/string/creation.spec.ts @@ -12,3 +12,48 @@ test("StringStream can be created via static from method", (t) => { t.true(stringStream instanceof StringStream); }); + +test("StringStream split returns instance of StringStream", (t) => { + const stringStream = StringStream.from(["1", "2", "3", "4"]); + const newStream = stringStream.split("2"); + + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); + t.deepEqual(newStream.constructor.name, "StringStream"); +}); + +test("StringStream map returns instance of StringStream", (t) => { + const stringStream = StringStream.from(["1", "2", "3", "4"]); + const newStream = stringStream.map(chunk => `foo-${chunk}`); + + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); + t.deepEqual(newStream.constructor.name, "StringStream"); +}); + +test("StringStream flatMap returns instance of StringStream", (t) => { + const stringStream = StringStream.from(["1", "2", "3", "4"]); + const newStream = stringStream.flatMap(chunk => [chunk]); + + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); + t.deepEqual(newStream.constructor.name, "StringStream"); +}); + +test("StringStream filter returns instance of StringStream", (t) => { + const stringStream = StringStream.from(["1", "2", "3", "4"]); + const newStream = stringStream.filter(chunk => parseInt(chunk, 10) % 2 === 0); + + // This is kind of tricky since static code analysis shows that "stringStream.filter" returns + // DataString (as declared in method return type) + // but on runtime, it internally returns "this" which is StringStream + // + // So it can be solved like: + // + // StringStream.filter( + // callback: TransformFunction, + // ...args: W): StringStream + // { + // return super.filter(callback, ...args) as StringStream; + // } + + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); + t.deepEqual(newStream.constructor.name, "StringStream"); +}); From d89dad7acd37f2bbc0c70ef584dac0920295e454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Tue, 2 Nov 2021 12:17:41 +0000 Subject: [PATCH 2/4] Add PoC for 'generic-this' --- src/generic-this-1.ts | 171 +++++++++++++++++++++++++++ test/streams/string/creation.spec.ts | 16 +++ 2 files changed, 187 insertions(+) create mode 100644 src/generic-this-1.ts diff --git a/src/generic-this-1.ts b/src/generic-this-1.ts new file mode 100644 index 0000000..ff74c68 --- /dev/null +++ b/src/generic-this-1.ts @@ -0,0 +1,171 @@ +// Run with "npm run build && node build/src/generic-this-1.js" +// This is a manual approach (could use some sort of code generation too). + +class BaseClass { + public value: T; + + constructor(value: T) { + this.value = value; + } + + create(value: T): BaseClass; + create(value: U): BaseClass; + create(value: U): BaseClass { + return new BaseClass(value); + } + + method1(): this { + return this; + } + + method2(): BaseClass { + return this; + } + + method3(value: T): BaseClass { + return this.create(value); + } + + method5(value: T): BaseClass; + method5(value: U): BaseClass; + method5(value: U): BaseClass { + return this.create(value); + } +} + +class DerivedClass extends BaseClass { + create(value: T): DerivedClass; + create(value: U): DerivedClass; + create(value: U): DerivedClass { + return new DerivedClass(value); + } + + method2(): DerivedClass { + return super.method2() as DerivedClass; + } + + method3(value: T): DerivedClass { + return super.method3(value) as DerivedClass; + } + + method5(value: T): DerivedClass; + method5(value: U): DerivedClass; + method5(value: U): DerivedClass { + return super.method5(value) as DerivedClass; + } + + ownMethod1() { + console.log(this); + } +} + +class DerivedClassFixedType extends DerivedClass { + create(value: number): DerivedClassFixedType { + return new DerivedClassFixedType(value); + } + + method2(): DerivedClassFixedType { + return super.method2() as DerivedClassFixedType; + } + + method3(value: number): DerivedClassFixedType { + return super.method3(value) as DerivedClassFixedType; + } + + method5(value: number): DerivedClassFixedType { + return super.method5(value) as DerivedClassFixedType; + } + + ownMethod2() { + console.log(this); + } +} + +class BaseClassFixedType extends BaseClass { + create(value: string): BaseClassFixedType { + return new BaseClassFixedType(value); + } + + method2(): BaseClassFixedType { + return super.method2() as BaseClassFixedType; + } + + method3(value: string): BaseClassFixedType { + return super.method3(value) as BaseClassFixedType; + } + + method5(value: string): BaseClassFixedType { + return super.method5(value) as BaseClassFixedType; + } + + ownMethod3() { + console.log(this); + } +} + +// --- BaseClass + +const bcString = new BaseClass("foo"); +const bcString1 = bcString.method1(); +const bcString2 = bcString.method2(); +const bcString3 = bcString.method3("bar"); +const bcString5 = bcString.method5(123); + +for (const instance of [bcString, bcString1, bcString2, bcString3, bcString5]) { + console.log(`instanceof: ${ instance instanceof BaseClass }; constructor.name: ${ instance.constructor.name }`); +} + +// --- DerivedClass + +const dcString = new DerivedClass("foo"); +const dcString1 = dcString.method1(); +const dcString2 = dcString.method2(); +const dcString3 = dcString.method3("bar"); +const dcString5 = dcString.method5(123); + +for (const instance of [dcString, dcString1, dcString2, dcString3, dcString5]) { + console.log(`instanceof: ${ instance instanceof DerivedClass }; constructor.name: ${ instance.constructor.name }`); +} + +dcString.ownMethod1(); +dcString1.ownMethod1(); +dcString2.ownMethod1(); +dcString3.ownMethod1(); +dcString5.ownMethod1(); + +// --- DerivedClassFixedType + +const dcftString = new DerivedClassFixedType(123); +const dcftString1 = dcftString.method1(); +const dcftString2 = dcftString.method2(); +const dcftString3 = dcftString.method3(456); +const dcftString5 = dcftString.method5(123); + +for (const instance of [dcftString, dcftString1, dcftString2, dcftString3, dcftString5]) { + console.log(`instanceof: ${ instance instanceof DerivedClassFixedType }; constructor.name: ${ instance.constructor.name }`); +} + +dcftString.ownMethod2(); +dcftString1.ownMethod2(); +dcftString2.ownMethod2(); +dcftString3.ownMethod2(); +dcftString5.ownMethod2(); + +// --- BaseClassFixedType + +const bcftString = new BaseClassFixedType("foo"); +const bcftString1 = bcftString.method1(); +const bcftString2 = bcftString.method2(); +const bcftString3 = bcftString.method3("bar"); +const bcftString5 = bcftString.method5("baz"); +// const toBcStrign6 = bcftString.method5(123); // This won't work for now + +for (const instance of [bcftString, bcftString1, bcftString2, bcftString3, bcftString5]) { + console.log(`instanceof: ${ instance instanceof BaseClassFixedType }; constructor.name: ${ instance.constructor.name }`); +} + +bcftString.ownMethod3(); +bcftString1.ownMethod3(); +bcftString2.ownMethod3(); +bcftString3.ownMethod3(); +bcftString5.ownMethod3(); diff --git a/test/streams/string/creation.spec.ts b/test/streams/string/creation.spec.ts index 5d2a9ad..5ad4712 100644 --- a/test/streams/string/creation.spec.ts +++ b/test/streams/string/creation.spec.ts @@ -17,6 +17,10 @@ test("StringStream split returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.split("2"); + console.log("split", newStream); // returns DataStream due to .asNewFlattenedStream call + // newStream.split("3"); // static code analysis accpets that but it throws + // during runtime since newStream is DataStream + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); }); @@ -25,6 +29,10 @@ test("StringStream map returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.map(chunk => `foo-${chunk}`); + console.log("map", newStream); // returns StringStream (since it returns this internally) + // newStream.split("3"); // static code analysis does not accept that since + // map should return DataStream (but it works at runtime) + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); }); @@ -33,6 +41,10 @@ test("StringStream flatMap returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.flatMap(chunk => [chunk]); + console.log("flatMap", newStream); // returns DataStream due to .asNewFlattenedStream call + // newStream.split("3"); // static code analysis does not accept that since + // map should return DataStream (an it throws at runtime) + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); }); @@ -41,6 +53,10 @@ test("StringStream filter returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.filter(chunk => parseInt(chunk, 10) % 2 === 0); + console.log("filter", newStream); // returns StringStream (since it returns this internally) + // newStream.split("3"); // static code analysis does not accept that since + // map should return DataStream (but it works at runtime) + // This is kind of tricky since static code analysis shows that "stringStream.filter" returns // DataString (as declared in method return type) // but on runtime, it internally returns "this" which is StringStream From eb28e588c45683dc6433bfc98290ebc31f7534d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 3 Nov 2021 08:31:26 +0000 Subject: [PATCH 3/4] Fix static and runtime return type for StringSplit for inherited methods This commit is trying to solve the issue of "polymorphic generic this as a return type". For more in-depth description see: https://github.com/scramjetorg/scramjet-dev/pull/28. This issue is related to a fact that in `DataStream` class we have multiple methods (transfroms) which returns `DataStream` (so the instance of the same class, but with different type parameter). This works fine for `DataStream` itself but causes problems for derived classes as a return type is not dynamic and inhertied methods called on an instance of derived class will always return the instance of a parent class. First change was to introduce `.create()` method which is used inside tramsforms directly instead of calling `new DataStream...`. This allows derived classes to override it and have control over how its instances are created. With `.create()` in use, new instance based on call context (current this) is created. This results with a correct return type during runtime. The second change was to override transforms signatures in the derived class so correct return type could be declared. Those overriden transforms call `super.transform(...)` internally so the logic stays exactly the same. This solves the issue of TS not correctly recognizing return type (as without it, method signatures declare base class as return type). Above works well for generic derived classes. However, for non-generic derived classes (like `StringStream` in our case), overridden transform signatures (and also `.create()` method) needs to be non-generic as well to work with this approach. And so the third change was to overload transforms in a base class to have both non-generic and generic versions. --- src/streams/data-stream.ts | 14 ++++++- src/streams/string-stream.ts | 19 ++++++++- test/streams/string/creation.spec.ts | 60 ++++++++++++++-------------- 3 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/streams/data-stream.ts b/src/streams/data-stream.ts index 0d28516..e0a4e86 100644 --- a/src/streams/data-stream.ts +++ b/src/streams/data-stream.ts @@ -50,6 +50,12 @@ export class DataStream implements BaseStream, AsyncIterable { }; } + create(): DataStream; + create(): DataStream; + create(): DataStream { + return new DataStream(); + } + map(callback: TransformFunction, ...args: W): DataStream { if (args?.length) { this.ifca.addTransform(this.injectArgsToCallback(callback, args)); @@ -70,6 +76,8 @@ export class DataStream implements BaseStream, AsyncIterable { return this; } + flatMap(callback: TransformFunction, W>, ...args: W): DataStream; + flatMap(callback: TransformFunction, W>, ...args: W): DataStream flatMap(callback: TransformFunction, W>, ...args: W): DataStream { return this.asNewFlattenedStream(this.map, W>(callback, ...args)); } @@ -214,7 +222,9 @@ export class DataStream implements BaseStream, AsyncIterable { fromStream: W, onEndYield?: () => { yield: boolean, value?: U } ): DataStream { - return DataStream.from((async function * (stream){ + const newStream = this.create(); + + newStream.read((async function * (stream){ for await (const chunks of stream) { yield* chunks; } @@ -227,6 +237,8 @@ export class DataStream implements BaseStream, AsyncIterable { } } })(fromStream)); + + return newStream; } protected getReader( diff --git a/src/streams/string-stream.ts b/src/streams/string-stream.ts index 42543bd..6e24e60 100644 --- a/src/streams/string-stream.ts +++ b/src/streams/string-stream.ts @@ -1,18 +1,33 @@ import { DataStream } from "./data-stream"; -import { AnyIterable } from "../types"; +import { AnyIterable, TransformFunction } from "../types"; export class StringStream extends DataStream { + create(): StringStream { + return new StringStream(); + } + split(splitBy: string) { const splitter = this.getSplitter(splitBy); const onEndYield = () => ({ yield: splitter.emitLastValue, value: splitter.lastValue }); - return this.asNewFlattenedStream>>( + return this.asNewFlattenedStream( this.map>(splitter.fn), onEndYield ) as StringStream; } + filter(callback: TransformFunction, ...args: W): StringStream { + return super.filter(callback, ...args) as StringStream; + } + + flatMap( + callback: TransformFunction, W>, + ...args: W + ): StringStream { + return super.flatMap(callback, ...args) as StringStream; + } + private getSplitter(splitBy: string) { const result: any = { emitLastValue: false, diff --git a/test/streams/string/creation.spec.ts b/test/streams/string/creation.spec.ts index 5ad4712..9c5d8fe 100644 --- a/test/streams/string/creation.spec.ts +++ b/test/streams/string/creation.spec.ts @@ -17,59 +17,61 @@ test("StringStream split returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.split("2"); - console.log("split", newStream); // returns DataStream due to .asNewFlattenedStream call - // newStream.split("3"); // static code analysis accpets that but it throws - // during runtime since newStream is DataStream + console.log("split", newStream); t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); -}); -test("StringStream map returns instance of StringStream", (t) => { - const stringStream = StringStream.from(["1", "2", "3", "4"]); - const newStream = stringStream.map(chunk => `foo-${chunk}`); + const newStream2 = newStream.split("1"); - console.log("map", newStream); // returns StringStream (since it returns this internally) - // newStream.split("3"); // static code analysis does not accept that since - // map should return DataStream (but it works at runtime) + console.log("split", newStream2); - t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); - t.deepEqual(newStream.constructor.name, "StringStream"); + t.true(newStream2 instanceof StringStream, `Should be an instance of StringStream, not ${newStream2.constructor.name}`); + t.deepEqual(newStream2.constructor.name, "StringStream"); }); test("StringStream flatMap returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.flatMap(chunk => [chunk]); - console.log("flatMap", newStream); // returns DataStream due to .asNewFlattenedStream call - // newStream.split("3"); // static code analysis does not accept that since - // map should return DataStream (an it throws at runtime) + console.log("flatMap", newStream); t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); + + const newStream2 = newStream.split("1"); + + console.log("split", newStream2); + + t.true(newStream2 instanceof StringStream, `Should be an instance of StringStream, not ${newStream2.constructor.name}`); + t.deepEqual(newStream2.constructor.name, "StringStream"); }); test("StringStream filter returns instance of StringStream", (t) => { const stringStream = StringStream.from(["1", "2", "3", "4"]); const newStream = stringStream.filter(chunk => parseInt(chunk, 10) % 2 === 0); - console.log("filter", newStream); // returns StringStream (since it returns this internally) + console.log("filter", newStream); + + t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); + t.deepEqual(newStream.constructor.name, "StringStream"); + + const newStream2 = newStream.split("1"); + + console.log("split", newStream2); + + t.true(newStream2 instanceof StringStream, `Should be an instance of StringStream, not ${newStream2.constructor.name}`); + t.deepEqual(newStream2.constructor.name, "StringStream"); +}); + +test("StringStream map returns instance of StringStream", (t) => { + const stringStream = StringStream.from(["1", "2", "3", "4"]); + const newStream = stringStream.map(chunk => `foo-${chunk}`); + + console.log("map", newStream); // returns StringStream (since it returns this internally) // newStream.split("3"); // static code analysis does not accept that since // map should return DataStream (but it works at runtime) - // This is kind of tricky since static code analysis shows that "stringStream.filter" returns - // DataString (as declared in method return type) - // but on runtime, it internally returns "this" which is StringStream - // - // So it can be solved like: - // - // StringStream.filter( - // callback: TransformFunction, - // ...args: W): StringStream - // { - // return super.filter(callback, ...args) as StringStream; - // } - t.true(newStream instanceof StringStream, `Should be an instance of StringStream, not ${newStream.constructor.name}`); t.deepEqual(newStream.constructor.name, "StringStream"); }); From e05c53080751ae5a482d88a75d475c451c2a0cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 10 Nov 2021 08:36:50 +0000 Subject: [PATCH 4/4] Move 'generic this' sample to samples folder --- {src => samples}/generic-this-1.ts | 6 ++++-- tsconfig.build.json | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) rename {src => samples}/generic-this-1.ts (95%) diff --git a/src/generic-this-1.ts b/samples/generic-this-1.ts similarity index 95% rename from src/generic-this-1.ts rename to samples/generic-this-1.ts index ff74c68..643cb9c 100644 --- a/src/generic-this-1.ts +++ b/samples/generic-this-1.ts @@ -1,5 +1,7 @@ -// Run with "npm run build && node build/src/generic-this-1.js" -// This is a manual approach (could use some sort of code generation too). +// This is a sample file demonstarting the approached used for "polymorphic this" +// for generic method inheritance. It can be run with: +// +// npm run build && node build/samples/generic-this-1.js" class BaseClass { public value: T; diff --git a/tsconfig.build.json b/tsconfig.build.json index c08846e..8f9f870 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -12,6 +12,7 @@ "include": [ "src", "test", - "bdd" + "bdd", + "samples" ] }