diff --git a/polymod/hscript/_internal/Expr.hx b/polymod/hscript/_internal/Expr.hx index eb70bf84..8a4b5c2a 100644 --- a/polymod/hscript/_internal/Expr.hx +++ b/polymod/hscript/_internal/Expr.hx @@ -160,6 +160,7 @@ enum Error EInvalidIterator(v:String); EInvalidOp(op:String); EInvalidAccess(f:String); + EPrivateField(f:String); EInvalidModule(m:String); EBlacklistedModule(m:String); EBlacklistedField(f:String); @@ -180,6 +181,7 @@ enum Error EClassInvalidSuper; // Accessing "super" in a parentless class EScriptThrow(v:Dynamic); // Script called "throw" EScriptCallThrow(v:Dynamic); // Script called a function which threw + EInvalidAccessorCombination(accessors:Array); // Fallback error type. ECustom(msg:String); } diff --git a/polymod/hscript/_internal/Interp.hx b/polymod/hscript/_internal/Interp.hx index cc24326c..92fb7284 100644 --- a/polymod/hscript/_internal/Interp.hx +++ b/polymod/hscript/_internal/Interp.hx @@ -27,6 +27,7 @@ import polymod.util.Util; import haxe.PosInfos; import haxe.Constraints.IMap; +using Lambda; using StringTools; private enum Stop @@ -76,6 +77,8 @@ class Interp var curExpr:Expr; #end + var inPrivateAccess:Bool = false; + function getClassDecl():Null { if (_classDeclOverride != null) @@ -123,23 +126,35 @@ class Interp return Type.createInstance(defaultVariables.get(cl), args); } + function tryBuildClass(clsRef:PolymodStaticClassReference, args:Array):Null + { + if (clsRef.cls != getClassDecl() && !clsRef.canInstantiate) + { + error(ECustom('Cannot access private constructor of "${clsRef.cls.name}"')); + return null; + } + return clsRef.instantiate(args); + } + // Try to retrieve a scripted class with this name in the same package. if (getClassDecl().pkg != null && getClassDecl().pkg.length > 0) { var localClassId = getClassDecl().pkg.join('.') + "." + cl; var clsRef = PolymodStaticClassReference.tryBuild(localClassId); - if (clsRef != null) return clsRef.instantiate(args); + if (clsRef != null) return tryBuildClass(clsRef, args); } // Try to retrieve a scripted class with this name in the base package. var clsRef = PolymodStaticClassReference.tryBuild(cl); - if (clsRef != null) return clsRef.instantiate(args); + if (clsRef != null) return tryBuildClass(clsRef, args); + @:privateAccess if (getClassDecl().imports != null && getClassDecl().imports.exists(cl)) { var clsRef = PolymodStaticClassReference.tryBuild(getClassDecl().imports.get(cl).fullPath); - if (clsRef != null) return clsRef.instantiate(args); + if (clsRef != null) return tryBuildClass(clsRef, args); } + @:privateAccess if (getClassDecl()?.pkg != null) { @@ -148,7 +163,15 @@ class Interp if (_scriptClassDescriptors.exists(packagedClass)) { // OVERRIDE CHANGE: Create a PolymodScriptClass instead of a ScriptClass - var proxy:PolymodAbstractScriptClass = new PolymodScriptClass(_scriptClassDescriptors.get(packagedClass), args); + var clsDescriptor:ClassDecl = findScriptClassDescriptor(packagedClass); + var ctorField:Null = clsDescriptor.fields.find((f) -> f.name == 'new'); + if (clsDescriptor != getClassDecl() && ctorField?.access.contains(APrivate)) + { + error(ECustom('Cannot access private constructor of ${clsRef.cls.name}')); + return null; + } + + var proxy:PolymodAbstractScriptClass = new PolymodScriptClass(clsDescriptor, args); return proxy; } } @@ -159,7 +182,14 @@ class Interp if (_scriptClassDescriptors.exists(importedClass.fullPath)) { // OVERRIDE CHANGE: Create a PolymodScriptClass instead of a ScriptClass - var proxy:PolymodAbstractScriptClass = new PolymodScriptClass(_scriptClassDescriptors.get(importedClass.fullPath), args); + var clsDescriptor:ClassDecl = findScriptClassDescriptor(importedClass.fullPath); + var ctorField:Null = clsDescriptor.fields.find((f) -> f.name == 'new'); + if (clsDescriptor != getClassDecl() && ctorField?.access.contains(APrivate)) + { + error(ECustom('Cannot access private constructor of ${clsRef.cls.name}')); + return null; + } + var proxy:PolymodAbstractScriptClass = new PolymodScriptClass(clsDescriptor, args); return proxy; } @@ -262,6 +292,15 @@ class Interp // Force call super function. return o.scriptCallSuper(f, args); } + + #if POLYMOD_STRICT_SYNTAX + if (!checkPrivateAccess(o, f)) + { + error(EPrivateField(f)); + return null; + } + #end + else if (Std.isOfType(o, PolymodStaticAbstractReference)) { var ref:PolymodStaticAbstractReference = cast(o, PolymodStaticAbstractReference); @@ -776,9 +815,7 @@ class Interp return assignValue(e1, expr(e2)); } - function assignValue(e1:Expr, - v:Dynamic, - _abstractInlineAssign:Bool = false):Null + function assignValue(e1:Expr, v:Dynamic, _abstractInlineAssign:Bool = false):Null { switch (Tools.expr(e1)) { @@ -2006,8 +2043,23 @@ class Interp restore(old); return val; } - case EMeta(_, _, e): - return expr(e); + case EMeta(name, args, e): + switch (name) + { + case ':privateAccess': + // Check to make sure we aren't already in a private access block. + // Otherwise the state'll be overwritten and the original block won't work anymore. + if (!inPrivateAccess) + { + inPrivateAccess = true; + var obj = expr(e); // We evaulate the expression knowing we're in a private access. + inPrivateAccess = false; + return obj; + } + return expr(e); + default: + return expr(e); + } case ECheckType(e, _): return expr(e); } @@ -2432,6 +2484,104 @@ class Interp return null; } + function checkPrivateAccess(o:Dynamic, f:String):Bool + { + // If we're in a private access block, automatically allow it. + if (inPrivateAccess) + return true; + + // First, script classes. + if (Std.isOfType(o, PolymodStaticClassReference)) + { + var ref:PolymodStaticClassReference = cast(o, PolymodStaticClassReference); + + // We are retrieving this field within the same script class context. + if (o.cls == getClassDecl()) + return true; + + var field:Null = PolymodScriptClass.scriptInterp.getScriptClassStaticFieldDecl(ref.getFullyQualifiedName(), f); + if (field != null && field.access.contains(APrivate)) + { + return false; + } + } + else if (Std.isOfType(o, PolymodScriptClass)) + { + var ref:PolymodAbstractScriptClass = cast(o, PolymodAbstractScriptClass); + + if (ref.fullyQualifiedName == getClassFullyQualifiedName()) + return true; + + // If this script class shares the same superclasses with this class we're in (inheritance) then we can access it if the field is any of those. + var superClasses:Array = PolymodScriptClass.getSuperClasses(getClassDecl()) ?? []; + var inheritatedSuperClasses:Array = [ref.fullyQualifiedName].concat(PolymodScriptClass.getSuperClasses(ref._c)).filter((superCls:String) -> return superClasses.contains(superCls)); + + var superClass:Dynamic = ref.superClass; + while (superClass != null) + { + if (Std.isOfType(superClass, PolymodScriptClass)) + { + var scriptCls:PolymodScriptClass = cast(superClass, PolymodScriptClass); + if (inheritatedSuperClasses.contains(scriptCls.fullyQualifiedName)) + { + var fieldDecl = scriptCls.findField(f); + if (fieldDecl != null && fieldDecl.access.contains(APrivate)) + { + return true; + } + } + superClass = superClass.superClass; + } + else + { + var superClsName:String = Util.getTypeNameOf(superClass); + if (inheritatedSuperClasses.contains(superClsName)) + { + if (PolymodFinalMacro.getPrivateFieldsOf(superClsName).contains(f)) + { + return true; + } + } + superClass = Type.getSuperClass(superClass); + } + } + + // Regular check the script class itself has within the script class itself. + var fieldDecl:Null = ref.findField(f); + if (fieldDecl != null && fieldDecl.access.contains(APrivate)) + { + return false; + } + } + else if (Std.isOfType(o, PolymodStaticAbstractReference)) + { + // Abstracts can't have private instance fields. + return true; + } + else + { + // We're checking for fields from within a regular class. + var superClasses:Array = PolymodScriptClass.getSuperClasses(getClassDecl()) ?? []; + var inheritatedSuperClasses:Array = [Util.getTypeNameOf(o)].concat(Util.getSuperClasses(o) ?? []).filter((superCls:String) -> return superClasses.contains(superCls)); + + for (cls in inheritatedSuperClasses) + { + if (PolymodFinalMacro.getPrivateFields(cls).contains(f)) + { + return true; + } + } + + // Check from within the class itself. + if (PolymodFinalMacro.getPrivateFieldsOf(o).contains(f)) + { + return false; + } + } + + return true; + } + function get(o:Dynamic, f:String):Null { if (o == null) error(ENullObjectReference(f)); @@ -2447,6 +2597,14 @@ class Interp var oCls:String = Util.getTypeNameOf(o); #if hl oCls = oCls.replace('$', ''); #end + #if POLYMOD_STRICT_SYNTAX + if (!checkPrivateAccess(o, f)) + { + error(EPrivateField(f)); + return null; + } + #end + // Check if the field is a blacklisted static field. if (PolymodScriptClass.blacklistedStaticFields.exists(o) && PolymodScriptClass.blacklistedStaticFields.get(o).contains(f)) { @@ -2563,6 +2721,14 @@ class Interp var oCls:String = Util.getTypeNameOf(o); #if hl oCls = oCls.replace('$', ''); #end + #if POLYMOD_STRICT_SYNTAX + if (!checkPrivateAccess(o, f)) + { + error(EPrivateField(f)); + return null; + } + #end + // Check if the field is a blacklisted static field. if (PolymodScriptClass.blacklistedStaticFields.exists(o) && PolymodScriptClass.blacklistedStaticFields.get(o).contains(f)) { diff --git a/polymod/hscript/_internal/Parser.hx b/polymod/hscript/_internal/Parser.hx index e2a66fb7..0853ddcd 100644 --- a/polymod/hscript/_internal/Parser.hx +++ b/polymod/hscript/_internal/Parser.hx @@ -104,6 +104,8 @@ class Parser var idents:Array; var uid:Int = 0; + var packageSet:Bool = false; + #if hscriptPos var origin:String; var tokenMin:Int; @@ -1463,6 +1465,13 @@ class Parser switch (ident) { case "package": + #if POLYMOD_STRICT_SYNTAX + if (!packageSet) + packageSet = true; + else + error(ECustom("Unknown identifier: package"), currentPos, currentPos); // Throw an error if there was a package already set. + #end + var path = parsePath(); ensure(TSemicolon); return DPackage(path); @@ -1664,9 +1673,17 @@ class Parser case "override": access.push(AOverride); case "public": - access.push(APublic); + // Throw an error if the user tries declaring a variable as public when it's already been declared private. + if (access.contains(APrivate)) + error(ECustom("Conflicting access modifier public"), currentPos, currentPos); + else if (!access.contains(APublic)) + access.push(APublic); case "private": - access.push(APrivate); + // Throw an error if the user tries declaring a variable as private when it's already been declared public. + if (access.contains(APublic)) + error(ECustom("Conflicting access modifier private"), currentPos, currentPos); + else if (!access.contains(APrivate)) + access.push(APrivate); case "inline": access.push(AInline); case "static": @@ -1674,6 +1691,11 @@ class Parser case "macro": access.push(AMacro); case "function": + if (access.contains(AOverride) && access.contains(AStatic)) + { + error(EInvalidAccessorCombination(['override', 'static']), currentPos, currentPos); + } + var name = getIdent(); var inf = parseFunctionDecl(); maybe(TSemicolon); @@ -1714,6 +1736,13 @@ class Parser else ensure(TSemicolon); + #if POLYMOD_STRICT_SYNTAX + if (access.contains(AInline) && !access.contains(AStatic)) + { + error(ECustom('Invalid modifier: inline on non-static variable'), currentPos, currentPos); + } + #end + return { name: name, meta: meta, diff --git a/polymod/hscript/_internal/PolymodFinalMacro.hx b/polymod/hscript/_internal/PolymodFinalMacro.hx index b7bac476..0392f91c 100644 --- a/polymod/hscript/_internal/PolymodFinalMacro.hx +++ b/polymod/hscript/_internal/PolymodFinalMacro.hx @@ -1,5 +1,6 @@ package polymod.hscript._internal; +import haxe.macro.Type.ClassField; #if macro import haxe.macro.Context; import haxe.macro.Expr; @@ -15,6 +16,34 @@ class PolymodFinalMacro static inline final METADATA_RESOURCE_NAME:String = 'PolymodFinalMacro_METADATA'; #if !macro + private static var _allFinals:Null>> = null; + private static var _allPrivateProperties:Null>> = null; + private static var _allPrivatesFields:Null>> = null; + + public static function getAllFinals():Map> + { + if (_allFinals == null) _allFinals = PolymodFinalMacro.fetchAllFinals(); + return _allFinals; + } + + public static function getAllPrivateProperties():Map> + { + if (_allPrivateProperties == null) _allPrivateProperties = PolymodFinalMacro.fetchAllPrivateProperties(); + return _allPrivateProperties; + } + + public static function getAllPrivateFields():Map> + { + #if POLYMOD_STRICT_SYNTAX + if (_allPrivatesFields == null) _allPrivatesFields = PolymodFinalMacro.fetchAllPrivateFields(); + #else + // Disable private fields. + if (_allPrivatesFields == null) _allPrivatesFields = []; + #end + + return _allPrivatesFields; + } + public static inline function getFinals(fullPath:String):Array { return getAllFinals().get(fullPath) ?? []; } @@ -39,23 +68,21 @@ class PolymodFinalMacro return result; } - private static var _allFinals:Null>> = null; - - public static function getAllFinals():Map> - { - if (_allFinals == null) _allFinals = PolymodFinalMacro.fetchAllFinals(); - return _allFinals; + public static inline function getPrivateFields(fullPath:String):Array { + return getAllPrivateFields().get(fullPath) ?? []; } - private static var _allPrivates:Null>> = null; + public static inline function getPrivateFieldsOf(obj:Dynamic):Array { + while (Std.isOfType(obj, PolymodScriptClass)) obj = obj.superClass; - public static function getAllPrivateProperties():Map> - { - if (_allPrivates == null) _allPrivates = PolymodFinalMacro.fetchAllPrivateProperties(); - return _allPrivates; + var typeName:String = polymod.util.Util.getTypeNameOf(obj); + var result = getPrivateFields(typeName); + return result; } #end + static var calledBefore:Bool = false; + public static macro function locateAllFinals():Void { Context.onAfterTyping((types) -> { @@ -64,7 +91,8 @@ class PolymodFinalMacro var startTime:Float = Sys.time(); var allFinals:Array = []; - var allPrivates:Array = []; + var allPrivateProperties:Array = []; + var allPrivateFields:Array = []; for (type in types) { @@ -84,13 +112,21 @@ class PolymodFinalMacro allFinals.push(entryData); } - var privates:Array = listPrivateFields(fields); - if (privates.length > 0) + var privatesProperties:Array = listPrivateProperties(fields); + if (privatesProperties.length > 0) { - var entryData:Array = [classPath, privates]; - allPrivates.push(entryData); + var entryData:Array = [classPath, privatesProperties]; + allPrivateProperties.push(entryData); } + #if POLYMOD_STRICT_SYNTAX + var privateFields:Array = listPrivateFields(fields); + if (privateFields.length > 0) + { + var entryData:Array = [classPath, privateFields]; + allPrivateFields.push(entryData); + } + #end default: continue; } @@ -98,7 +134,8 @@ class PolymodFinalMacro var metadataHXSF = haxe.Serializer.run({ finals: allFinals, - privates: allPrivates + privateProperties: allPrivateProperties, + privateFields: allPrivateFields, }); Context.addResource(METADATA_RESOURCE_NAME, haxe.io.Bytes.ofString(metadataHXSF)); @@ -108,7 +145,8 @@ class PolymodFinalMacro Context.info('PolymodFinalMacro: ' + 'Detected ${allFinals.length} classes with final variables, ' - + '${allPrivates.length} classes with (default,null) properties ' + + '${allPrivateProperties.length} classes with (default,null) properties, ' + + '${allPrivateFields.length} classes with private variables ' + 'in ${duration} sec.', Context.currentPos()); @@ -165,7 +203,7 @@ class PolymodFinalMacro return result; } - static function listPrivateFields(fields:Array):Array + static function listPrivateProperties(fields:Array):Array { var result:Array = []; @@ -183,7 +221,16 @@ class PolymodFinalMacro return result; } - static var calledBefore:Bool = false; + static function listPrivateFields(fields:Array):Array + { + var result:Array = []; + for (field in fields) + { + if (field.isPublic) continue; + result.push(field.name); + } + return result; + } #end public static function fetchAllFinals():Map> @@ -216,7 +263,7 @@ class PolymodFinalMacro public static function fetchAllPrivateProperties():Map> { var metaData = fetchMetadata(); - var privates:Array = cast metaData.privates; + var privates:Array = cast metaData.privateProperties; if (privates != null) { @@ -224,7 +271,7 @@ class PolymodFinalMacro for (element in privates) { - if (element.length != 2) throw 'Malformed element in privates: ' + element; + if (element.length != 2) throw 'Malformed element in privates properties: ' + element; var classPath:String = element[0]; var privates:Array = element[1]; @@ -240,6 +287,32 @@ class PolymodFinalMacro } } + public static function fetchAllPrivateFields():Map> + { + var metaData = fetchMetadata(); + var privateVars:Array = cast metaData.privateFields; + + if (privateVars != null) + { + var result:Map> = []; + + for (element in privateVars) + { + if (element.length != 2) throw 'Malformed element in private fields: ' + element; + + var classPath:String = element[0]; + var privates:Array = element[1]; + + result.set(classPath, privates); + } + return result; + } + else + { + throw 'No private fields found in PolymodFinalMacro'; + } + } + static var _metadata:Dynamic = null; static function fetchMetadata():Dynamic { diff --git a/polymod/hscript/_internal/PolymodScriptClass.hx b/polymod/hscript/_internal/PolymodScriptClass.hx index 2c9e8277..3504a381 100644 --- a/polymod/hscript/_internal/PolymodScriptClass.hx +++ b/polymod/hscript/_internal/PolymodScriptClass.hx @@ -399,6 +399,11 @@ class PolymodScriptClass Polymod.error(SCRIPT_PARSE_FAILED, 'Error while parsing class ${path}#${errLine}: EClassUnresolvedSuperclass' + '\n' + 'Unresolved superclass "${cls}", ${reason}', SCRIPT_RUNTIME); return false; + case EInvalidAccessorCombination(accessors): + Polymod.error( + SCRIPT_PARSE_FAILED, + 'Error while parsing function ${path}#${errLine}: EInvalidAccessorCombination' + '\n' + 'Invalid modifier combination: ${accessors.join(' + ')}', SCRIPT_RUNTIME); + return false; default: Polymod.error(SCRIPT_PARSE_FAILED, 'Error while parsing script ${path}#${errLine}: ' + '\n' + 'An unknown error occurred: ${err}', SCRIPT_RUNTIME); return false; @@ -789,6 +794,8 @@ class PolymodScriptClass createSuperClass(args); } _constructorArgs = args; + + validateClassFields(); } var __superClassFieldList:Array = null; @@ -896,8 +903,10 @@ class PolymodScriptClass superClass = Type.createInstance(clsToCreate, args); } + } - // Throw an error if the script class has an instance field with the same name as one from the super class. + private function validateClassFields():Void + { for (f in _c.fields) { switch (f.kind) @@ -905,10 +914,39 @@ class PolymodScriptClass case KVar(v): if (!f.access.contains(AStatic) && superHasField(f.name)) { - throw 'Redefinition of variable "${f.name}" from superclass not allowed'; + // Throw an error if the script class has an instance field with the same name as one from the super class. + throw 'Redefinition of variable "${f.name}" from superclass not allowed.'; + } + case KFunction(fn): + #if POLYMOD_STRICT_SYNTAX + if (f.access.contains(AOverride) && !superHasField(f.name)) + { + // Throw an error if a function is declared overwritten but isn't overriding anything. + throw "Field " + f.name + " is declared 'override' but doesn't override any field."; } + else if (!f.access.contains(AOverride) && superHasField(f.name)) + { + var superClassPackage:String = ''; + if (superClass is PolymodScriptClass) + { + superClassPackage = Util.getFullClassName((cast superClass : PolymodScriptClass)._c); + } + else + { + // TODO: Fetch entire package name? + superClassPackage = Util.getTypeNameOf(superClass); + } - case _: + // Throw an error if a function is overriden but doesn't have the override accessor. + throw "Field " + f.name + " should be declared with 'override' since it is inherited from superclass " + superClassPackage + '.'; + } + else if (f.access.contains(AOverride) && superClass == null) + { + // Throw an error if the override accessor is used with no super class. + throw "Invalid modifier: override on field" + f.name + " of class that has no parent."; + } + #end + default: } } } diff --git a/polymod/hscript/_internal/PolymodStaticClassReference.hx b/polymod/hscript/_internal/PolymodStaticClassReference.hx index d660bfeb..1a4c0e5c 100644 --- a/polymod/hscript/_internal/PolymodStaticClassReference.hx +++ b/polymod/hscript/_internal/PolymodStaticClassReference.hx @@ -3,6 +3,7 @@ package polymod.hscript._internal; import polymod.hscript._internal.Expr; import polymod.util.Util; +using Lambda; using StringTools; /** @@ -14,6 +15,14 @@ class PolymodStaticClassReference { public var cls:Null; + public var canInstantiate(get, never):Bool; + + public function get_canInstantiate():Bool + { + var ctorField = cls.fields.find((f) -> f.name == 'new'); + return !ctorField.access.contains(APrivate); + } + public function new(?cls:ClassDecl) { this.cls = cls; diff --git a/polymod/hscript/_internal/Printer.hx b/polymod/hscript/_internal/Printer.hx index c7e33356..2a9dc7ff 100644 --- a/polymod/hscript/_internal/Printer.hx +++ b/polymod/hscript/_internal/Printer.hx @@ -683,8 +683,8 @@ class Printer if (m.params != null) { output += "("; - for (i in 0...m.params.length) - { + for (i in 0...m.params.length) + { var param:Expr = m.params[i]; output += this.exprToString(param); if (i < m.params.length - 1) output += ", "; @@ -726,6 +726,7 @@ class Printer case EInvalidIterator(v): 'Invalid iterator "$v".'; case EInvalidOp(op): 'Invalid operator "$op".'; case EInvalidAccess(f): 'Invalid access to field "$f".'; + case EPrivateField(f): 'Cannot access private field "$f".'; case EInvalidModule(m): 'Invalid module "$m".'; case EBlacklistedModule(m): 'Blacklisted module "$m".'; case EBlacklistedField(m): 'Blacklisted field "$m".'; @@ -747,6 +748,7 @@ class Printer // TODO: Do we need to distinguish these? case EScriptCallThrow(v): 'Script threw an exception:\n$v'; case EScriptThrow(v): 'User script threw an exception:\n$v'; + case EInvalidAccessorCombination(accessors): 'Invalid modifier combination: ${accessors.join(' + ')}'; case ECustom(msg): msg; }; #if hscriptPos diff --git a/polymod/util/Util.hx b/polymod/util/Util.hx index bfcbd46b..4c9fc409 100644 --- a/polymod/util/Util.hx +++ b/polymod/util/Util.hx @@ -736,6 +736,23 @@ class Util return errorMessage; } + public static function getSuperClasses(obj:Dynamic):Array + { + var cls = Type.getClass(obj) ?? Type.resolveClass(getTypeNameOf(obj)); + if (cls == null) return []; + + var superCls:Dynamic = Type.getSuperClass(cls); + if (superCls == null) return []; + + var superClassList:Array = []; + while (superCls != null) + { + superClassList.push(Type.getClassName(superCls)); + superCls = Type.getSuperClass(superCls); + } + return superClassList; + } + public static function getTypeNameOf(obj:Dynamic):String { var type = Type.typeof(obj);