Skip to content

Commit cdc3d35

Browse files
authored
Fix generic behaviour inheritance (#117)
* Fix AS inheritance * Remove comment * Fix import checks on code publish
1 parent 7ee5030 commit cdc3d35

File tree

6 files changed

+73
-29
lines changed

6 files changed

+73
-29
lines changed

src/Project/functions/createProjectData.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export function createProjectData(
4040
const projectData: ProjectData = {
4141
tsConfigPath,
4242
includePath,
43+
isSkippingPackages: projectOptions.skipPackages,
4344
isPackage,
4445
isPublishing: projectOptions.publish,
4546
logTruthyChanges,

src/Shared/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export interface ProjectOptions {
3939

4040
export interface ProjectData {
4141
includePath: string;
42+
isSkippingPackages: boolean;
4243
isPackage: boolean;
4344
isPublishing: boolean;
4445
logTruthyChanges: boolean;

src/TSTransformer/classes/TransformState.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export class TransformState {
9292
return this._context === CompliationContext.Shared;
9393
}
9494

95-
public get isPublish() {
95+
public isPublish() {
9696
return this.data.isPublishing;
9797
}
9898

src/TSTransformer/nodes/class/transformClassConstructor.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import { transformIdentifierDefined } from "TSTransformer/nodes/expressions/tran
88
import { transformParameters } from "TSTransformer/nodes/transformParameters";
99
import { transformPropertyName } from "TSTransformer/nodes/transformPropertyName";
1010
import { transformStatementList } from "TSTransformer/nodes/transformStatementList";
11-
import { isRootAirshipBehaviourClass, isRootAirshipSingletonClass } from "TSTransformer/util/extendsAirshipBehaviour";
11+
import {
12+
isAirshipSingletonClass,
13+
isRootAirshipBehaviourClass,
14+
isRootAirshipSingletonClass,
15+
} from "TSTransformer/util/extendsAirshipBehaviour";
1216
import { getExtendsNode } from "TSTransformer/util/getExtendsNode";
1317
import { getStatements } from "TSTransformer/util/getStatements";
1418
import ts from "typescript";
@@ -67,9 +71,10 @@ export function transformClassConstructor(
6771

6872
let bodyStatements = originNode ? getStatements(originNode.body) : [];
6973

70-
const isAirshipSingleton = isRootAirshipSingletonClass(state, node);
71-
const isAirshipBehaviour = isRootAirshipBehaviourClass(state, node);
72-
let removeFirstSuper = isAirshipBehaviour || isAirshipSingleton;
74+
const isRootSingletonClass = isRootAirshipSingletonClass(state, node);
75+
const isAirshipSingleton = isAirshipSingletonClass(state, node);
76+
const isRootAirshipBehaviour = isRootAirshipBehaviourClass(state, node);
77+
let removeFirstSuper = isRootAirshipBehaviour || isAirshipSingleton;
7378

7479
let parameters = luau.list.make<luau.AnyIdentifier>();
7580
let hasDotDotDot = false;
@@ -82,7 +87,7 @@ export function transformClassConstructor(
8287
luau.list.pushList(statements, paramStatements);
8388
parameters = constructorParams;
8489
hasDotDotDot = constructorHasDotDotDot;
85-
} else if (getExtendsNode(node) && !isAirshipBehaviour && !isAirshipSingleton) {
90+
} else if (getExtendsNode(node) && !isRootAirshipBehaviour && !isRootSingletonClass) {
8691
// if extends + no constructor:
8792
// - add ... to params
8893
// - add super.constructor(self, ...)
@@ -143,7 +148,7 @@ export function transformClassConstructor(
143148

144149
if (
145150
ts.isIdentifier(member.name) &&
146-
(isAirshipSingleton || isAirshipBehaviour) &&
151+
(isAirshipSingleton || isRootAirshipBehaviour) &&
147152
isAirshipBehaviourReserved(member.name.text)
148153
) {
149154
DiagnosticService.addDiagnostic(errors.noReservedAirshipIdentifier(member.name));
@@ -177,7 +182,7 @@ export function transformClassConstructor(
177182
}
178183
}
179184

180-
if (isAirshipSingleton) {
185+
if (isAirshipSingleton && !node.modifiers?.some(value => ts.isAbstractModifier(value))) {
181186
createAirshipSingletonBoilerplate(state, node, name, statements);
182187
}
183188

src/TSTransformer/nodes/statements/transformImportDeclaration.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { TransformState } from "TSTransformer";
55
import { transformVariable } from "TSTransformer/nodes/statements/transformVariableStatement";
66
import { cleanModuleName } from "TSTransformer/util/cleanModuleName";
77
import { createImportExpression } from "TSTransformer/util/createImportExpression";
8-
import { isAirshipSingletonType } from "TSTransformer/util/extendsAirshipBehaviour";
8+
import { isAirshipSingletonType, isClassInheritingSymbol } from "TSTransformer/util/extendsAirshipBehaviour";
99
import { getOriginalSymbolOfNode } from "TSTransformer/util/getOriginalSymbolOfNode";
1010
import { getSourceFileFromModuleSpecifier } from "TSTransformer/util/getSourceFileFromModuleSpecifier";
1111
import { isSymbolOfValue } from "TSTransformer/util/isSymbolOfValue";
@@ -76,23 +76,24 @@ function shouldSkipSingletonImport(
7676
symbol: ts.Symbol,
7777
) {
7878
const linkedModuleSingletonIds = state.airshipBuildState.singletonTypes.get(module.fileName);
79-
if (!linkedModuleSingletonIds) {
79+
if (state.isPublish() && !linkedModuleSingletonIds) {
8080
return false;
8181
}
8282

83-
if (!symbol.valueDeclaration) return false;
83+
const valueDeclaration = symbol.valueDeclaration;
84+
if (!valueDeclaration) return false;
8485

8586
// get the value type of the import
86-
const valueType = state.typeChecker.getTypeAtLocation(symbol!.valueDeclaration!);
87+
const valueType = state.typeChecker.getTypeAtLocation(valueDeclaration);
8788

8889
if (!valueType) {
8990
return false;
9091
}
9192

9293
const typeUniqueId = state.airshipBuildState.getUniqueIdForType(state, valueType, module);
9394

94-
// Need to ensure we keep the import or strip it... Don't like this TBH
95-
if (!linkedModuleSingletonIds.has(typeUniqueId)) {
95+
if (state.isPublish() && !linkedModuleSingletonIds!.has(typeUniqueId)) {
96+
// Need to ensure we keep the import or strip it... Don't like this TBH
9697
return false;
9798
}
9899

@@ -101,6 +102,13 @@ function shouldSkipSingletonImport(
101102
// Ensure we're only using only a macro on the singleton
102103
let shouldSkip = true;
103104
ts.forEachChildRecursively(importDeclaration.getSourceFile(), node => {
105+
// If we have an inheriting singleton, we need to call the constructor of the base singleton
106+
if (ts.isClassLike(node) && isClassInheritingSymbol(state, node, symbol)) {
107+
shouldSkip = false;
108+
return "skip";
109+
}
110+
111+
// Any static accesses of singletons
104112
if (ts.isPropertyAccessExpression(node) && isStaticAirshipSingletonPropertyAccess(state, node, typeName)) {
105113
shouldSkip = false;
106114
return "skip";

src/TSTransformer/util/extendsAirshipBehaviour.ts

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ import { getExtendsNode } from "TSTransformer/util/getExtendsNode";
66
import { getOriginalSymbolOfNode } from "TSTransformer/util/getOriginalSymbolOfNode";
77
import ts from "typescript";
88

9+
export function isRootAirshipBehaviourClassNoState(
10+
singletonSymbol: ts.Symbol,
11+
typeChecker: ts.TypeChecker,
12+
node: ts.ClassLikeDeclaration,
13+
) {
14+
const extendsNode = getExtendsNode(node);
15+
if (extendsNode) {
16+
const symbol = getOriginalSymbolOfNode(typeChecker, extendsNode.expression);
17+
if (symbol === singletonSymbol) {
18+
return true;
19+
}
20+
}
21+
22+
return false;
23+
}
24+
925
export function isRootAirshipBehaviourClass(state: TransformState, node: ts.ClassLikeDeclaration) {
1026
const extendsNode = getExtendsNode(node);
1127
if (extendsNode) {
@@ -25,10 +41,11 @@ export function isRootAirshipSingletonClass(state: TransformState, node: ts.Clas
2541
if (extendsNode) {
2642
const airshipSingletonSymbol = state.services.airshipSymbolManager.getAirshipSingletonSymbolOrThrow();
2743

28-
const symbol = getOriginalSymbolOfNode(state.typeChecker, extendsNode.expression);
29-
if (symbol === airshipSingletonSymbol) {
30-
return true;
31-
}
44+
return isRootAirshipBehaviourClassNoState(airshipSingletonSymbol, state.typeChecker, node);
45+
// const symbol = getOriginalSymbolOfNode(state.typeChecker, extendsNode.expression);
46+
// if (symbol === airshipSingletonSymbol) {
47+
// return true;
48+
// }
3249
}
3350

3451
return false;
@@ -86,7 +103,7 @@ export function isAirshipSingletonClassNoState(
86103
}
87104

88105
// Get the root inheriting symbol (Should match AirshipBehaviour for this to be "extending" AirshipBehaviour)
89-
const baseTypeDeclaration = inheritance[inheritance.length - 1];
106+
const baseTypeDeclaration = inheritance[inheritance.length - 2] ?? inheritance[inheritance.length - 1];
90107
if (baseTypeDeclaration !== undefined) {
91108
return baseTypeDeclaration === airshipBehaviourSymbol;
92109
}
@@ -102,6 +119,7 @@ export function isAirshipSingletonClass(state: TransformState, node: ts.ClassLik
102119

103120
// check if the immediate extends is AirshipBehaviour
104121
let type = state.typeChecker.getTypeAtLocation(node);
122+
105123
if (type.isNullableType()) {
106124
type = type.getNonNullableType();
107125
}
@@ -111,17 +129,11 @@ export function isAirshipSingletonClass(state: TransformState, node: ts.ClassLik
111129
return true;
112130
}
113131

114-
// Get the inheritance tree, otherwise
115-
const inheritance = getAncestorTypeSymbols(type, state.typeChecker);
116-
if (inheritance.length === 0) {
117-
return false;
118-
}
132+
const extendsClasses = getTypesOfClasses(state.typeChecker, getExtendsClasses(state.typeChecker, node));
133+
if (extendsClasses.length === 0) return false;
119134

120-
// Get the root inheriting symbol (Should match AirshipBehaviour for this to be "extending" AirshipBehaviour)
121-
const baseTypeDeclaration = inheritance[inheritance.length - 1];
122-
if (baseTypeDeclaration !== undefined) {
123-
return baseTypeDeclaration === airshipBehaviourSymbol;
124-
}
135+
const baseClass = extendsClasses[extendsClasses.length - 2] ?? extendsClasses[extendsClasses.length - 1];
136+
return baseClass.symbol === airshipBehaviourSymbol;
125137
}
126138

127139
return false;
@@ -167,6 +179,23 @@ export function isAirshipBehaviourType(state: TransformState, type: ts.Type) {
167179
}
168180
}
169181

182+
export const enum SingletonQueryType {
183+
IsRootSingleton,
184+
IsAnySingleton,
185+
}
186+
187+
export function isClassInheritingSymbol(state: TransformState, node: ts.ClassLikeDeclaration, symbol: ts.Symbol) {
188+
const type = state.typeChecker.getTypeAtLocation(node);
189+
190+
// Get the inheritance tree, otherwise
191+
const inheritance = getAncestorTypeSymbols(type, state.typeChecker);
192+
if (inheritance.length === 0) {
193+
return false;
194+
}
195+
196+
return inheritance.some(value => value === symbol);
197+
}
198+
170199
export function isAirshipSingletonType(state: TransformState, type: ts.Type) {
171200
const airshipBehaviourSymbol = state.services.airshipSymbolManager.getAirshipSingletonSymbolOrThrow();
172201

0 commit comments

Comments
 (0)