Skip to content

Commit 24b9140

Browse files
committed
Error on invalid get component uses
1 parent 8914ba9 commit 24b9140

File tree

4 files changed

+101
-8
lines changed

4 files changed

+101
-8
lines changed

src/Shared/diagnostics.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,28 @@ export const errors = {
251251
];
252252
}),
253253

254+
unityMacroExpectsAirshipComponentTypeArgument: errorWithContext((type: string, isUnityObjectType: boolean) => {
255+
if (isUnityObjectType) {
256+
return [
257+
`${type} is a Unity Component, not an Airship Component`,
258+
suggestion("Change GetAirshipComponent to GetComponent<" + type + ">()"),
259+
];
260+
} else {
261+
return [`${type} is not a derived type of AirshipBehaviour`];
262+
}
263+
}),
264+
265+
unityMacroExpectsComponentTypeArgument: errorWithContext((type: string, isAirshipBehaviourType: boolean) => {
266+
if (isAirshipBehaviourType) {
267+
return [
268+
`${type} is an Airship Component, not a Unity Component`,
269+
suggestion("Change GetComponent to GetAirshipComponent<" + type + ">()"),
270+
];
271+
} else {
272+
return [`${type} is not a derived type of Component`];
273+
}
274+
}),
275+
254276
decoratorParamsLiteralsOnly: error(
255277
"Airship Behaviour decorators only accept literal `string`, `boolean` or `number` values",
256278
),
@@ -334,7 +356,7 @@ export const warnings = {
334356
"please remove this from the file as it will not be maintained in future.",
335357
} satisfies ts.Diagnostic,
336358

337-
genericBehaviourRefernece: warningWithContext(() => {
359+
genericBehaviourReference: warningWithContext(() => {
338360
return [
339361
"Generic AirshipBehaviours cannot be exposed to the inspector",
340362
suggestion(

src/TSTransformer/macros/propertyCallMacros.ts

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import { assert } from "Shared/util/assert";
44
import { DiagnosticService } from "TSTransformer/classes/DiagnosticService";
55
import { TransformState } from "TSTransformer/classes/TransformState";
66
import { MacroList, PropertyCallMacro } from "TSTransformer/macros/types";
7+
import { isUnityObjectType } from "TSTransformer/util/airshipBehaviourUtils";
78
import { convertToIndexableExpression } from "TSTransformer/util/convertToIndexableExpression";
9+
import { isAirshipBehaviourType, isAirshipBehaviourTypeNode } from "TSTransformer/util/extendsAirshipBehaviour";
810
import { isUsedAsStatement } from "TSTransformer/util/isUsedAsStatement";
911
import { offset } from "TSTransformer/util/offset";
1012
import { isDefinitelyType, isNumberType, isStringType } from "TSTransformer/util/types";
@@ -933,6 +935,44 @@ const PROMISE_METHODS: MacroList<PropertyCallMacro> = {
933935
}),
934936
};
935937

938+
const expectAirshipComponentGeneric = (propertyCallMacro: PropertyCallMacro, index: 0 = 0): PropertyCallMacro => {
939+
return (state, node, expression, args) => {
940+
if (node.typeArguments) {
941+
const typeNode = node.typeArguments[index];
942+
if (!isAirshipBehaviourTypeNode(state, typeNode)) {
943+
DiagnosticService.addDiagnostic(
944+
errors.unityMacroExpectsAirshipComponentTypeArgument(
945+
node,
946+
state.typeChecker.typeToString(state.typeChecker.getTypeFromTypeNode(node.typeArguments[0])),
947+
isUnityObjectType(state, state.getType(typeNode)),
948+
),
949+
);
950+
}
951+
}
952+
953+
return propertyCallMacro(state, node, expression, args);
954+
};
955+
};
956+
957+
const expectUnityComponentGeneric = (propertyCallMacro: PropertyCallMacro, index: 0 = 0): PropertyCallMacro => {
958+
return (state, node, expression, args) => {
959+
if (node.typeArguments) {
960+
const type = state.getType(node.typeArguments[index]);
961+
if (!isUnityObjectType(state, type)) {
962+
DiagnosticService.addDiagnostic(
963+
errors.unityMacroExpectsComponentTypeArgument(
964+
node,
965+
state.typeChecker.typeToString(state.typeChecker.getTypeFromTypeNode(node.typeArguments[0])),
966+
isAirshipBehaviourType(state, type, true),
967+
),
968+
);
969+
}
970+
}
971+
972+
return propertyCallMacro(state, node, expression, args);
973+
};
974+
};
975+
936976
const makeTypeArgumentAsStringMacro =
937977
(method: string, requiresArgument = true, defaultTypeName?: string): PropertyCallMacro =>
938978
(state, node, expression, args) => {
@@ -971,21 +1011,34 @@ const makeTypeArgumentAsStringMacro =
9711011
}
9721012
};
9731013

974-
const UNITY_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {
1014+
const COMPONENT_MACROS: MacroList<PropertyCallMacro> = {
9751015
GetComponent: makeTypeArgumentAsStringMacro("GetComponent"),
976-
GetAirshipComponent: makeTypeArgumentAsStringMacro("GetAirshipComponent"),
9771016
GetComponents: makeTypeArgumentAsStringMacro("GetComponents"),
978-
GetAirshipComponents: makeTypeArgumentAsStringMacro("GetAirshipComponents"),
9791017
AddComponent: makeTypeArgumentAsStringMacro("AddComponent"),
1018+
GetComponentInChildren: makeTypeArgumentAsStringMacro("GetComponentInChildren"),
1019+
GetComponentsInChildren: makeTypeArgumentAsStringMacro("GetComponentsInChildren"),
9801020
GetComponentInParent: makeTypeArgumentAsStringMacro("GetComponentInParent"),
1021+
};
1022+
1023+
const AIRSHIP_COMPONENT_MACROS: MacroList<PropertyCallMacro> = {
1024+
GetAirshipComponent: makeTypeArgumentAsStringMacro("GetAirshipComponent"),
1025+
GetAirshipComponents: makeTypeArgumentAsStringMacro("GetAirshipComponents"),
9811026
AddAirshipComponent: makeTypeArgumentAsStringMacro("AddAirshipComponent"),
982-
GetComponentsInChildren: makeTypeArgumentAsStringMacro("GetComponentsInChildren"),
9831027
GetAirshipComponentsInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentsInChildren"),
984-
GetComponentInChildren: makeTypeArgumentAsStringMacro("GetComponentInChildren"),
9851028
GetAirshipComponentInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentInChildren"),
9861029
GetAirshipComponentInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentInParent"),
9871030
GetAirshipComponentsInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentsInParent"),
9881031
};
1032+
1033+
const UNITY_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {};
1034+
1035+
for (const [macro, call] of Object.entries(AIRSHIP_COMPONENT_MACROS)) {
1036+
UNITY_GAMEOBJECT_METHODS[macro] = expectAirshipComponentGeneric(call);
1037+
}
1038+
for (const [macro, call] of Object.entries(COMPONENT_MACROS)) {
1039+
UNITY_GAMEOBJECT_METHODS[macro] = expectUnityComponentGeneric(call);
1040+
}
1041+
9891042
const UNITY_STATIC_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {
9901043
FindObjectOfType: makeTypeArgumentAsStringMacro("FindObjectOfType"),
9911044
FindObjectsByType: makeTypeArgumentAsStringMacro("FindObjectsByType"),

src/TSTransformer/util/airshipBehaviourUtils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export function getAncestorTypeSymbols(nodeType: ts.Type, typeChecker: ts.TypeCh
7171
nodeType = nodeType.getNonNullableType();
7272
}
7373

74+
if (!nodeType.symbol) return [];
75+
7476
const baseTypes = nodeType.getBaseTypes();
7577

7678
if (baseTypes) {

src/TSTransformer/util/extendsAirshipBehaviour.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export function isAirshipBehaviourProperty(state: TransformState, node: ts.Prope
140140
// Disallow generic properties
141141
const nodeTypeRef = node.type;
142142
if (nodeTypeRef && ts.isTypeReferenceNode(nodeTypeRef) && nodeTypeRef.typeArguments) {
143-
DiagnosticService.addDiagnostic(warnings.genericBehaviourRefernece(node));
143+
DiagnosticService.addDiagnostic(warnings.genericBehaviourReference(node));
144144
return false;
145145
}
146146
}
@@ -151,9 +151,11 @@ export function isAirshipBehaviourProperty(state: TransformState, node: ts.Prope
151151
return false;
152152
}
153153

154-
export function isAirshipBehaviourType(state: TransformState, type: ts.Type) {
154+
export function isAirshipBehaviourType(state: TransformState, type: ts.Type, includeBaseType = false) {
155155
const airshipBehaviourSymbol = state.services.airshipSymbolManager.getAirshipBehaviourSymbolOrThrow();
156156

157+
if (includeBaseType && airshipBehaviourSymbol === type.symbol) return true;
158+
157159
// Get the inheritance tree, otherwise
158160
const inheritance = getAncestorTypeSymbols(type, state.typeChecker);
159161
if (inheritance.length === 0) {
@@ -165,6 +167,8 @@ export function isAirshipBehaviourType(state: TransformState, type: ts.Type) {
165167
if (baseTypeDeclaration !== undefined) {
166168
return baseTypeDeclaration === airshipBehaviourSymbol;
167169
}
170+
171+
return false;
168172
}
169173

170174
export function isAirshipSingletonType(state: TransformState, type: ts.Type) {
@@ -203,3 +207,15 @@ export function isAirshipSingletonSymbol(state: TransformState, symbol: ts.Symbo
203207

204208
return false;
205209
}
210+
211+
export function isAirshipBehaviourTypeNode(state: TransformState, typeNode: ts.TypeNode) {
212+
// We'll try the quick symbol way
213+
const type = state.getType(typeNode);
214+
if (isAirshipBehaviourType(state, type, true)) return true; // quick and dirty
215+
216+
// If not, we'll use the slower value declaration walking method
217+
const symbolOfTypeArg = state.typeChecker.getTypeAtLocation(typeNode).symbol;
218+
const valueDeclaration = symbolOfTypeArg.valueDeclaration;
219+
if (!valueDeclaration || !ts.isClassDeclaration(valueDeclaration)) return false;
220+
return isAirshipBehaviourClass(state, valueDeclaration);
221+
}

0 commit comments

Comments
 (0)