Skip to content

Re-enable checks for final access. #415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EliteMasterEric wants to merge 12 commits into
base: experimental
Choose a base branch
from
Open

Re-enable checks for final access. #415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bfed94d
Improve error messages for invalid final access, plus some other fixes.
EliteMasterEric Jun 10, 2026
11e759c
Disable some annoying checkstyle rules (for now...)
EliteMasterEric Jun 10, 2026
28bbca4
Improve handling of properties with `null` or `never` setters (Haxe o…
EliteMasterEric Jun 10, 2026
c1dbcbd
Enable `locateAllFinals()` macro
EliteMasterEric Jun 10, 2026
ba2cdee
Further restricting final and null/never access
EliteMasterEric Jun 10, 2026
a4843fc
Fixed constructor assignment not working 100% as intended.
EliteMasterEric Jun 12, 2026
a15e39e
Fix unfenced import causing compile errors
NotHyper-474 Jun 11, 2026
011745d
Missed a spot for the constructor fix.
EliteMasterEric Jun 12, 2026
a5260e8
Remove some unused fields
EliteMasterEric Jun 13, 2026
d19b7aa
Add the stack trace for uncaught exceptions in script constructors.
EliteMasterEric Jun 13, 2026
2e60630
scriptInit now returns Null<T> for when the constructor fails.
EliteMasterEric Jun 13, 2026
23fe9e2
Write the list of finals/private properties to a resource file to pre…
EliteMasterEric Jun 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions checkstyle.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@
"ignoreOverride": true,
"tokens": ["ABSTRACT_DEF", "CLASS_DEF", "ENUM_DEF", "INTERFACE_DEF"],
"modifier": "PUBLIC",
"excludeNames": ["new", "toString"]
"excludeNames": ["new", "toString"],

"severity": "IGNORE"
},
"type": "FieldDocComment"
},
Expand Down Expand Up @@ -494,7 +496,9 @@
"props": {
"enforceReturnTypeForAnonymous": false,
"allowEmptyReturn": true,
"enforceReturnType": true
"enforceReturnType": true,

"severity": "IGNORE"
},
"type": "Return"
},
Expand Down
4 changes: 1 addition & 3 deletions include.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,5 @@
<haxelib name="thx.semver" />

<haxeflag name="--macro" value="polymod.hscript._internal.HScriptRedirectDefines.run()" if="POLYMOD_REDIRECT_HSCRIPT" />
<!--
<haxeflag name="- -macro" value="polymod.hscript._internal.PolymodFinalMacro.locateAllFinals()" />
-->
<haxeflag name="--macro" value="polymod.hscript._internal.PolymodFinalMacro.locateAllFinals()" />
</project>
11 changes: 8 additions & 3 deletions polymod/hscript/_internal/HScriptedClassMacro.hx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ class HScriptedClassMacro
{
// Context.info(' Building scripted class init() function', Context.currentPos());
var clsTypeName:String = cls.pack.join('.') != '' ? '${cls.pack.join('.')}.${cls.name}' : cls.name;
// scriptInit returns null if the script throws an error while initializing.
var clsType:ComplexType = Context.toComplexType(Context.getType(clsTypeName));
var function_init:Field =
{
name: 'scriptInit',
Expand All @@ -157,7 +159,7 @@ class HScriptedClassMacro
args: [
{name: 'clsName', type: macro :String}, {name: 'args', type: macro :...Dynamic}],
params: null,
ret: Context.toComplexType(Context.getType(clsTypeName)),
ret: polymod.util.MacroUtil.nullable(clsType),
expr: macro
{
var clsRef = polymod.hscript._internal.PolymodStaticClassReference.tryBuild(clsName);
Expand Down Expand Up @@ -185,8 +187,11 @@ class HScriptedClassMacro
}
catch (error)
{
var callStack:String = polymod.util.Util.fetchCallStack();

polymod.Polymod.error(SCRIPT_RUNTIME_EXCEPTION,
'Could not construct instance of scripted class (${clsName} extends ' + $v{clsTypeName} + '):\n${error}', SCRIPT_RUNTIME);
'An uncaught exception was thrown while constructing an instance of scripted class (${clsName} extends ' + $v{clsTypeName} + '):\n$error\n$callStack',
SCRIPT_RUNTIME);
return null;
}
},
Expand Down Expand Up @@ -524,7 +529,7 @@ class HScriptedClassMacro
{
if (_asc == null)
{
return $v{'PolymodScriptedClass<${cls.name} extends ${cls.superClass.t.get().name}>(NO ASC)'};
return $v{'PolymodScriptClass<${cls.name} extends ${cls.superClass.t.get().name}>(NO ASC)'};
}
else
{
Expand Down
171 changes: 83 additions & 88 deletions polymod/hscript/_internal/Interp.hx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,11 @@ class Interp
}
catch (error)
{
Polymod.error(SCRIPT_RUNTIME_EXCEPTION, 'Could not construct instance of scripted class ($clsToInit extends ' + clsName + '):\n${error}');
var callStack:String = polymod.util.Util.fetchCallStack();

Polymod.error(SCRIPT_RUNTIME_EXCEPTION,
'An uncaught exception was thrown while constructing an instance of scripted class ($clsToInit extends ' + clsName + '):\n$error\n$callStack',
SCRIPT_RUNTIME);
return null;
}
}
Expand Down Expand Up @@ -508,7 +512,7 @@ class Interp
}
}

private static function registerScriptClass(c:ClassDecl)
static function registerScriptClass(c:ClassDecl)
{
var name = Util.getFullClassName(c);

Expand Down Expand Up @@ -674,10 +678,10 @@ class Interp
{
var superClass:PolymodAbstractScriptClass = cast(_proxy.superClass, PolymodScriptClass);
return superClass.fieldWrite(id, v);
} else {
set(_proxy.superClass, id, v);
return v;
}

Reflect.setProperty(_proxy.superClass, id, v);
return v;
}

// Fallback to setting in local scope.
Expand Down Expand Up @@ -747,7 +751,9 @@ class Interp
return superClass.fieldWrite(id, v);
}

Reflect.setProperty(_proxy.superClass, id, v);
// Directly assign the value.
// This is needed because `assignValue` may sometimes be called from the constructor.
PolymodAbstractScriptClass.setClassObjectField(_proxy.superClass, id, v);
return v;
}

Expand All @@ -759,6 +765,7 @@ class Interp
switch (decl?.set)
{
case "set":
// Allow assigning to "null" only for local fields.
final setName = 'set_$id';
if (!_propTrack.exists(setName))
{
Expand All @@ -771,11 +778,16 @@ class Interp
case "never":
error(EInvalidPropSet(id));
return null;

case "null":
// If the property setter is "null", it can only be assigned on local fields.
// Thankfully, this is a local field!
// So we can just fallthrough to the default case.
}

if ((decl?.isfinal ?? false) && decl?.expr != null)
{
error(EInvalidAccess(id));
error(EInvalidFinalSet(id));
return null;
}
}
Expand All @@ -802,7 +814,9 @@ class Interp
return superClass.fieldWrite(id, v);
}

Reflect.setProperty(_proxy.superClass, id, v);
// Directly assign the value.
// This is needed because `assignValue` may sometimes be called from the constructor.
PolymodAbstractScriptClass.setClassObjectField(_proxy.superClass, id, v);
return v;
}
}
Expand All @@ -816,13 +830,21 @@ class Interp
for (imp in _proxy._c.imports)
{
if (imp.name != id0) continue;
var finals = PolymodFinalMacro.getAllFinals().get(imp.fullPath) ?? [];
var finals = PolymodFinalMacro.getFinals(imp.fullPath);

if (finals.contains(id))
{
error(EInvalidFinalSet(id));
return null;
}

var privates = PolymodFinalMacro.getPrivateProperties(imp.fullPath);

if (privates.contains(id))
{
error(EInvalidPropSet(id));
return null;
}
}
}
}
Expand Down Expand Up @@ -976,7 +998,7 @@ class Interp

var l = locals.get(id);
var v:Dynamic = (l == null) ? resolve(id) : l.r;
if (l != null && l.isfinal && l.r != null) return error(EInvalidAccess(id));
if (l != null && l.isfinal && l.r != null) return error(EInvalidFinalSet(id));
if (prefix)
{
v += delta;
Expand Down Expand Up @@ -1112,7 +1134,7 @@ class Interp
}
}

inline function error(e:#if hscriptPos ErrorDef #else Error #end, rethrow = false):Dynamic
public inline function error(e:#if hscriptPos ErrorDef #else Error #end, rethrow = false):Dynamic
{
#if hscriptPos var e = new Error(e, curExpr?.pmin ?? 0, curExpr?.pmax ?? 0, curExpr?.origin ?? 'unknown', curExpr?.line ?? 0); #end
if (rethrow) this.rethrow(e)
Expand Down Expand Up @@ -1480,7 +1502,7 @@ class Interp
{
case EField(e, f):
var obj = expr(e);
if (obj == null) error(EInvalidAccess(f));
if (obj == null) error(ENullObjectReference(f));
return fcall(obj, f, args);
default:
return call(null, expr(e), args);
Expand Down Expand Up @@ -2294,22 +2316,6 @@ class Interp
}

error(EInvalidScriptedVarGet(f));

// var result = Reflect.getProperty(o, f);
// To save a bit of performance, we only query for the existence of the property
// if the value is reported as null, AND only in debug builds.

// #if debug
// if (!Reflect.hasField(o, f))
// {
// var propertyList = Type.getInstanceFields(Type.getClass(o));
// if (propertyList.indexOf(f) == -1)
// {
// error(EInvalidScriptedVarGet(f));
// }
// }
// #end
// return result;
}
#if (hl && haxe4)
else if (Std.isOfType(o, Enum))
Expand Down Expand Up @@ -2348,23 +2354,6 @@ class Interp
}
}
#end
// if (o == null) error(EInvalidAccess(f));
// return
// {
// #if php
// // https://github.com/HaxeFoundation/haxe/issues/4915
// try
// {
// Reflect.getProperty(o, f);
// }
// catch (e:Dynamic)
// {
// Reflect.field(o, f);
// }
// #else
// Reflect.getProperty(o, f);
// #end
// }
}

function set(o:Dynamic, f:String, v:Dynamic):Dynamic
Expand Down Expand Up @@ -2402,14 +2391,21 @@ class Interp
}
catch (e:Dynamic)
{
error(EInvalidFinalSet(f));
error(EInvalidAccess(f));
}
}
else if (Std.isOfType(o, PolymodStaticClassReference))
{
var ref:PolymodStaticClassReference = cast(o, PolymodStaticClassReference);

return ref.setField(f, v);
try
{
return ref.setField(f, v);
}
catch (e:Dynamic)
{
error(EInvalidAccess(f));
}
}
else if (Std.isOfType(o, PolymodScriptClass))
{
Expand All @@ -2426,7 +2422,7 @@ class Interp
return superClass.fieldWrite(f, v);
}

Reflect.setProperty(proxy.superClass, f, v);
set(proxy.superClass, f, v);
}
else
{
Expand All @@ -2442,14 +2438,11 @@ class Interp
}

error(EInvalidScriptedVarSet(f));

// Reflect.setProperty(o, f, v);
// return v;
}

try
{
Reflect.setProperty(o, f, v);
PolymodAbstractScriptClass.setClassObjectField(o, f, v);
}
catch (e)
{
Expand Down Expand Up @@ -2910,45 +2903,47 @@ class Interp
var fieldDecl = getScriptClassStaticFieldDecl(clsName, fieldName);
if (fieldDecl != null)
{
if (!this.variables.exists(prefixedName))
switch (fieldDecl.kind)
{
switch (fieldDecl.kind)
{
case KFunction(_fn):
throw 'Cannot override function ${prefixedName}';
case KVar(v):
if (v.set != null)
case KFunction(_fn):
throw 'Cannot override function ${prefixedName}';
case KVar(v):
if (v.isfinal) {
throw 'Cannot override final static field ${prefixedName}';
}
if (v.set != null)
{
switch (v.set)
{
switch (v.set)
{
case 'set':
var setterFunc = 'set_${fieldName}';
if (hasScriptClassStaticFunction(clsName, setterFunc))
{
return callScriptClassStaticFunction(clsName, setterFunc, [value]);
}
else
{
throw 'Could not resolve setter for property ${prefixedName}';
}
case 'default':
this.variables.set(prefixedName, value);
return value;
default:
case 'set':
var setterFunc = 'set_${fieldName}';
if (hasScriptClassStaticFunction(clsName, setterFunc))
{
return callScriptClassStaticFunction(clsName, setterFunc, [value]);
}
else
{
throw 'Could not resolve setter for property ${prefixedName}';
}
}
else
{
this.variables.set(prefixedName, value);
return value;
}

case 'never':
throw 'Cannot assign to property ${prefixedName}';

case 'null':
throw 'Cannot assign to property ${prefixedName}';

case 'default':
this.variables.set(prefixedName, value);
return value;
default:
throw 'Could not resolve setter for property ${prefixedName}';
}
}
}
else
{
this.variables.set(prefixedName, value);
return value;
}
else
{
this.variables.set(prefixedName, value);
return value;
}
}
}
else
Expand Down Expand Up @@ -3011,4 +3006,4 @@ private class ArrayIterator<T>
{
return a[pos++];
}
}
}
Loading