Skip to content

Commit 2d77b49

Browse files
VorliasVorlias
committed
Add diagnostics for invalid macro uses
2 parents 6f897e6 + aafa22e commit 2d77b49

File tree

5 files changed

+210
-70
lines changed

5 files changed

+210
-70
lines changed

‎src/Shared/diagnostics.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,32 @@ export const errors = {
289289
];
290290
}),
291291

292+
unityMacroExpectsAirshipComponentTypeArgument: errorWithContext(
293+
(type: string, name: string, isUnityObjectType: boolean) => {
294+
if (isUnityObjectType) {
295+
return [
296+
`${type} is a Unity Component, not an Airship Component`,
297+
suggestion(`Change this call to ${name}<${type}>()`),
298+
];
299+
} else {
300+
return [`${type} is not a derived type of AirshipBehaviour`];
301+
}
302+
},
303+
),
304+
305+
unityMacroExpectsComponentTypeArgument: errorWithContext(
306+
(type: string, name: string, isAirshipBehaviourType: boolean) => {
307+
if (isAirshipBehaviourType) {
308+
return [
309+
`${type} is an Airship Component, not a Unity Component`,
310+
suggestion(`Change this call to ${name}<${type}>()`),
311+
];
312+
} else {
313+
return [`${type} is not a derived type of Component`];
314+
}
315+
},
316+
),
317+
292318
decoratorParamsLiteralsOnly: error(
293319
"Airship Behaviour decorators only accept literal `string`, `boolean` or `number` values. For RequireComponent, use `typeof(ComponentType)` syntax.",
294320
),
@@ -406,7 +432,7 @@ export const warnings = {
406432
"please remove this from the file as it will not be maintained in future.",
407433
} satisfies ts.Diagnostic,
408434

409-
genericBehaviourRefernece: warningWithContext(() => {
435+
genericBehaviourReference: warningWithContext(() => {
410436
return [
411437
"Generic AirshipBehaviours cannot be exposed to the inspector",
412438
suggestion(

‎src/TSTransformer/macros/airship/propertyCallMacros.ts‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import luau from "@roblox-ts/luau-ast";
2+
import { errors, warnings } from "Shared/diagnostics";
3+
import { DiagnosticService } from "TSTransformer/classes/DiagnosticService";
4+
import { MacroList, PropertyCallMacro } from "TSTransformer/macros/types";
5+
import { isUnityObjectType } from "TSTransformer/util/airshipBehaviourUtils";
6+
import { convertToIndexableExpression } from "TSTransformer/util/convertToIndexableExpression";
7+
import { isAirshipBehaviourType, isAirshipBehaviourTypeNode } from "TSTransformer/util/extendsAirshipBehaviour";
8+
import ts from "typescript";
9+
10+
const expectAirshipComponentGeneric = (
11+
name: string,
12+
propertyCallMacro: PropertyCallMacro,
13+
index: 0 = 0,
14+
): PropertyCallMacro => {
15+
return (state, node, expression, args) => {
16+
if (node.typeArguments) {
17+
const typeNode = node.typeArguments[index];
18+
if (!isAirshipBehaviourTypeNode(state, typeNode)) {
19+
DiagnosticService.addDiagnostic(
20+
errors.unityMacroExpectsAirshipComponentTypeArgument(
21+
node,
22+
state.typeChecker.typeToString(state.typeChecker.getTypeFromTypeNode(node.typeArguments[0])),
23+
getAirshipMacroAlternativeName(propertyCallMacro)!,
24+
isUnityObjectType(state, state.getType(typeNode)),
25+
),
26+
);
27+
}
28+
}
29+
30+
return propertyCallMacro(state, node, expression, args);
31+
};
32+
};
33+
34+
const expectUnityComponentGeneric = (
35+
name: string,
36+
propertyCallMacro: PropertyCallMacro,
37+
index: 0 = 0,
38+
): PropertyCallMacro => {
39+
return (state, node, expression, args) => {
40+
if (node.typeArguments) {
41+
const type = state.getType(node.typeArguments[index]);
42+
if (!isUnityObjectType(state, type)) {
43+
DiagnosticService.addDiagnostic(
44+
errors.unityMacroExpectsComponentTypeArgument(
45+
node,
46+
state.typeChecker.typeToString(state.typeChecker.getTypeFromTypeNode(node.typeArguments[0])),
47+
getAirshipMacroAlternativeName(propertyCallMacro)!,
48+
isAirshipBehaviourType(state, type, true),
49+
),
50+
);
51+
}
52+
}
53+
54+
return propertyCallMacro(state, node, expression, args);
55+
};
56+
};
57+
58+
const makeTypeArgumentAsStringMacro =
59+
(method: string, requiresArgument = true, defaultTypeName?: string): PropertyCallMacro =>
60+
(state, node, expression, args) => {
61+
let type: ts.Type | undefined;
62+
63+
if (node.typeArguments) {
64+
type = state.getType(node.typeArguments[0]);
65+
} else if (ts.isAsExpression(node.parent)) {
66+
type = state.getType(node.parent.type);
67+
DiagnosticService.addDiagnostic(
68+
warnings.unityMacroAsExpressionWarning(method, state.typeChecker.typeToString(type))(node.parent),
69+
);
70+
}
71+
72+
if (requiresArgument && !defaultTypeName && !type && args.length === 0) {
73+
DiagnosticService.addSingleDiagnostic(errors.unityMacroTypeArgumentRequired(node, method));
74+
}
75+
76+
if (type) {
77+
args.unshift(luau.string(state.typeChecker.typeToString(type)));
78+
return luau.create(luau.SyntaxKind.MethodCallExpression, {
79+
expression: convertToIndexableExpression(expression),
80+
name: method,
81+
args: luau.list.make(...args),
82+
});
83+
} else {
84+
if (defaultTypeName !== undefined) {
85+
args.unshift(luau.string(defaultTypeName));
86+
}
87+
88+
return luau.create(luau.SyntaxKind.MethodCallExpression, {
89+
expression: convertToIndexableExpression(expression),
90+
name: method,
91+
args: luau.list.make(...args),
92+
});
93+
}
94+
};
95+
96+
const COMPONENT_MACROS = {
97+
GetComponent: makeTypeArgumentAsStringMacro("GetComponent"),
98+
GetComponents: makeTypeArgumentAsStringMacro("GetComponents"),
99+
AddComponent: makeTypeArgumentAsStringMacro("AddComponent"),
100+
GetComponentInChildren: makeTypeArgumentAsStringMacro("GetComponentInChildren"),
101+
GetComponentsInChildren: makeTypeArgumentAsStringMacro("GetComponentsInChildren"),
102+
GetComponentInParent: makeTypeArgumentAsStringMacro("GetComponentInParent"),
103+
GetComponentsInParent: makeTypeArgumentAsStringMacro("GetComponentsInParent"),
104+
} satisfies MacroList<PropertyCallMacro>;
105+
106+
const AIRSHIP_COMPONENT_MACROS = {
107+
GetAirshipComponent: makeTypeArgumentAsStringMacro("GetAirshipComponent"),
108+
GetAirshipComponents: makeTypeArgumentAsStringMacro("GetAirshipComponents"),
109+
AddAirshipComponent: makeTypeArgumentAsStringMacro("AddAirshipComponent"),
110+
GetAirshipComponentsInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentsInChildren"),
111+
GetAirshipComponentInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentInChildren"),
112+
GetAirshipComponentInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentInParent"),
113+
GetAirshipComponentsInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentsInParent"),
114+
} satisfies MacroList<PropertyCallMacro>;
115+
116+
const ALTERNATIVE_NAMES: ReadonlyArray<
117+
[airship: PropertyCallMacro, unityName: keyof typeof COMPONENT_MACROS | keyof typeof AIRSHIP_COMPONENT_MACROS]
118+
> = [
119+
[COMPONENT_MACROS.AddComponent, "AddAirshipComponent"],
120+
[COMPONENT_MACROS.GetComponent, "GetAirshipComponent"],
121+
[COMPONENT_MACROS.GetComponentInChildren, "GetAirshipComponentInChildren"],
122+
[COMPONENT_MACROS.GetComponentInParent, "GetAirshipComponentInParent"],
123+
[COMPONENT_MACROS.GetComponents, "GetAirshipComponents"],
124+
[COMPONENT_MACROS.GetComponentsInChildren, "GetAirshipComponentsInChildren"],
125+
[COMPONENT_MACROS.GetComponentsInParent, "GetAirshipComponentsInParent"],
126+
127+
[AIRSHIP_COMPONENT_MACROS.AddAirshipComponent, "AddComponent"],
128+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponent, "GetComponent"],
129+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponentInChildren, "GetComponentInChildren"],
130+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponentInParent, "GetComponentInParent"],
131+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponents, "GetComponents"],
132+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponentsInChildren, "GetComponentsInChildren"],
133+
[AIRSHIP_COMPONENT_MACROS.GetAirshipComponentsInParent, "GetComponentsInParent"],
134+
];
135+
136+
export const UNITY_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {};
137+
for (const [macro, call] of Object.entries(AIRSHIP_COMPONENT_MACROS)) {
138+
UNITY_GAMEOBJECT_METHODS[macro] = expectAirshipComponentGeneric(macro, call);
139+
}
140+
for (const [macro, call] of Object.entries(COMPONENT_MACROS)) {
141+
UNITY_GAMEOBJECT_METHODS[macro] = expectUnityComponentGeneric(macro, call);
142+
}
143+
144+
export const UNITY_STATIC_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {
145+
FindObjectOfType: makeTypeArgumentAsStringMacro("FindObjectOfType"),
146+
FindObjectsByType: makeTypeArgumentAsStringMacro("FindObjectsByType"),
147+
};
148+
export const UNITY_COMPONENT_METHODS: MacroList<PropertyCallMacro> = {
149+
GetComponent: COMPONENT_MACROS.GetComponent,
150+
GetComponents: COMPONENT_MACROS.GetComponents,
151+
};
152+
export const UNITY_OBJECT_METHODS: MacroList<PropertyCallMacro> = {
153+
IsA: makeTypeArgumentAsStringMacro("IsA"),
154+
};
155+
156+
function getAirshipMacroAlternativeName(propertyMacro: PropertyCallMacro): string | undefined {
157+
return ALTERNATIVE_NAMES.find(f => f[0] === propertyMacro)?.[1];
158+
}

‎src/TSTransformer/macros/propertyCallMacros.ts‎

Lines changed: 5 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import luau from "@roblox-ts/luau-ast";
2-
import { errors, warnings } from "Shared/diagnostics";
32
import { assert } from "Shared/util/assert";
4-
import { DiagnosticService } from "TSTransformer/classes/DiagnosticService";
53
import { TransformState } from "TSTransformer/classes/TransformState";
4+
import {
5+
UNITY_COMPONENT_METHODS,
6+
UNITY_GAMEOBJECT_METHODS,
7+
UNITY_STATIC_GAMEOBJECT_METHODS,
8+
} from "TSTransformer/macros/airship/propertyCallMacros";
69
import { MacroList, PropertyCallMacro } from "TSTransformer/macros/types";
710
import { convertToIndexableExpression } from "TSTransformer/util/convertToIndexableExpression";
811
import { isUsedAsStatement } from "TSTransformer/util/isUsedAsStatement";
@@ -933,71 +936,6 @@ const PROMISE_METHODS: MacroList<PropertyCallMacro> = {
933936
}),
934937
};
935938

936-
const makeTypeArgumentAsStringMacro =
937-
(method: string, requiresArgument = true, defaultTypeName?: string): PropertyCallMacro =>
938-
(state, node, expression, args) => {
939-
let type: ts.Type | undefined;
940-
941-
if (node.typeArguments) {
942-
type = state.getType(node.typeArguments[0]);
943-
} else if (ts.isAsExpression(node.parent)) {
944-
type = state.getType(node.parent.type);
945-
DiagnosticService.addDiagnostic(
946-
warnings.unityMacroAsExpressionWarning(method, state.typeChecker.typeToString(type))(node.parent),
947-
);
948-
}
949-
950-
if (requiresArgument && !defaultTypeName && !type && args.length === 0) {
951-
DiagnosticService.addSingleDiagnostic(errors.unityMacroTypeArgumentRequired(node, method));
952-
}
953-
954-
if (type) {
955-
args.unshift(luau.string(state.typeChecker.typeToString(type)));
956-
return luau.create(luau.SyntaxKind.MethodCallExpression, {
957-
expression: convertToIndexableExpression(expression),
958-
name: method,
959-
args: luau.list.make(...args),
960-
});
961-
} else {
962-
if (defaultTypeName !== undefined) {
963-
args.unshift(luau.string(defaultTypeName));
964-
}
965-
966-
return luau.create(luau.SyntaxKind.MethodCallExpression, {
967-
expression: convertToIndexableExpression(expression),
968-
name: method,
969-
args: luau.list.make(...args),
970-
});
971-
}
972-
};
973-
974-
const UNITY_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {
975-
GetComponent: makeTypeArgumentAsStringMacro("GetComponent"),
976-
GetAirshipComponent: makeTypeArgumentAsStringMacro("GetAirshipComponent"),
977-
GetComponents: makeTypeArgumentAsStringMacro("GetComponents"),
978-
GetAirshipComponents: makeTypeArgumentAsStringMacro("GetAirshipComponents"),
979-
AddComponent: makeTypeArgumentAsStringMacro("AddComponent"),
980-
GetComponentInParent: makeTypeArgumentAsStringMacro("GetComponentInParent"),
981-
AddAirshipComponent: makeTypeArgumentAsStringMacro("AddAirshipComponent"),
982-
GetComponentsInChildren: makeTypeArgumentAsStringMacro("GetComponentsInChildren"),
983-
GetAirshipComponentsInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentsInChildren"),
984-
GetComponentInChildren: makeTypeArgumentAsStringMacro("GetComponentInChildren"),
985-
GetAirshipComponentInChildren: makeTypeArgumentAsStringMacro("GetAirshipComponentInChildren"),
986-
GetAirshipComponentInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentInParent"),
987-
GetAirshipComponentsInParent: makeTypeArgumentAsStringMacro("GetAirshipComponentsInParent"),
988-
};
989-
const UNITY_STATIC_GAMEOBJECT_METHODS: MacroList<PropertyCallMacro> = {
990-
FindObjectOfType: makeTypeArgumentAsStringMacro("FindObjectOfType"),
991-
FindObjectsByType: makeTypeArgumentAsStringMacro("FindObjectsByType"),
992-
};
993-
const UNITY_COMPONENT_METHODS: MacroList<PropertyCallMacro> = {
994-
GetComponent: makeTypeArgumentAsStringMacro("GetComponent"),
995-
GetComponents: makeTypeArgumentAsStringMacro("GetComponents"),
996-
};
997-
const UNITY_OBJECT_METHODS: MacroList<PropertyCallMacro> = {
998-
IsA: makeTypeArgumentAsStringMacro("IsA"),
999-
};
1000-
1001939
export const PROPERTY_CALL_MACROS: { [className: string]: MacroList<PropertyCallMacro> } = {
1002940
// math classes
1003941
// CFrame: makeMathSet("+", "-", "*"),

‎src/TSTransformer/util/airshipBehaviourUtils.ts‎

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

77+
if (!nodeType.symbol) return [];
78+
7779
const baseTypes = nodeType.getBaseTypes();
7880

7981
if (baseTypes) {

‎src/TSTransformer/util/extendsAirshipBehaviour.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ export function isAirshipBehaviourProperty(state: TransformState, node: ts.Prope
164164
// Disallow generic properties
165165
const nodeTypeRef = node.type;
166166
if (nodeTypeRef && ts.isTypeReferenceNode(nodeTypeRef) && nodeTypeRef.typeArguments) {
167-
DiagnosticService.addDiagnostic(warnings.genericBehaviourRefernece(node));
167+
DiagnosticService.addDiagnostic(warnings.genericBehaviourReference(node));
168168
return false;
169169
}
170170
}
@@ -175,9 +175,11 @@ export function isAirshipBehaviourProperty(state: TransformState, node: ts.Prope
175175
return false;
176176
}
177177

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

181+
if (includeBaseType && airshipBehaviourSymbol === type.symbol) return true;
182+
181183
// Get the inheritance tree, otherwise
182184
const inheritance = getAncestorTypeSymbols(type, state.typeChecker);
183185
if (inheritance.length === 0) {
@@ -189,6 +191,8 @@ export function isAirshipBehaviourType(state: TransformState, type: ts.Type) {
189191
if (baseTypeDeclaration !== undefined) {
190192
return baseTypeDeclaration === airshipBehaviourSymbol;
191193
}
194+
195+
return false;
192196
}
193197

194198
export const enum SingletonQueryType {
@@ -244,3 +248,15 @@ export function isAirshipSingletonSymbol(state: TransformState, symbol: ts.Symbo
244248

245249
return false;
246250
}
251+
252+
export function isAirshipBehaviourTypeNode(state: TransformState, typeNode: ts.TypeNode) {
253+
// We'll try the quick symbol way
254+
const type = state.getType(typeNode);
255+
if (isAirshipBehaviourType(state, type, true)) return true; // quick and dirty
256+
257+
// If not, we'll use the slower value declaration walking method
258+
const symbolOfTypeArg = state.typeChecker.getTypeAtLocation(typeNode).symbol;
259+
const valueDeclaration = symbolOfTypeArg.valueDeclaration;
260+
if (!valueDeclaration || !ts.isClassDeclaration(valueDeclaration)) return false;
261+
return isAirshipBehaviourClass(state, valueDeclaration);
262+
}

0 commit comments

Comments
 (0)