From 16615e3582ab52e8ed6de633b2adde38b54e8f88 Mon Sep 17 00:00:00 2001 From: Thakee Nathees Date: Wed, 14 Aug 2024 10:31:02 +0530 Subject: [PATCH 1/6] access check for symbols implemented --- .../passes/main/access_modifier_pass.py | 172 +++++++++++++----- .../passes/main/fuse_typeinfo_pass.py | 39 +++- jaclang/tests/test_language.py | 1 + 3 files changed, 155 insertions(+), 57 deletions(-) diff --git a/jaclang/compiler/passes/main/access_modifier_pass.py b/jaclang/compiler/passes/main/access_modifier_pass.py index ce826ad11..3fc8d1cae 100644 --- a/jaclang/compiler/passes/main/access_modifier_pass.py +++ b/jaclang/compiler/passes/main/access_modifier_pass.py @@ -3,19 +3,22 @@ This pass checks for access to attributes in the Jac language. """ -import os from typing import Optional import jaclang.compiler.absyntree as ast -from jaclang.compiler.constant import SymbolAccess +from jaclang.compiler.constant import SymbolAccess, SymbolType from jaclang.compiler.passes import Pass -from jaclang.compiler.symtable import SymbolTable +from jaclang.compiler.symtable import Symbol from jaclang.settings import settings class AccessCheckPass(Pass): """Jac Ast Access Check pass.""" + def report_error(self, message: str, node: Optional[ast.AstNode] = None) -> None: + """Report error message related to illegal access of attributes and objects.""" + self.error(message, node) + def after_pass(self) -> None: """After pass.""" pass @@ -26,44 +29,6 @@ def exit_node(self, node: ast.AstNode) -> None: if settings.lsp_debug and isinstance(node, ast.NameAtom) and not node.sym: self.warning(f"Name {node.sym_name} not present in symbol table") - def access_check(self, node: ast.Name) -> None: - """Access check.""" - node_info = ( - node.sym_tab.lookup(node.sym_name) - if isinstance(node.sym_tab, SymbolTable) - else None - ) - - if node.sym: - decl_package_path = os.path.dirname( - os.path.abspath(node.sym.defn[-1].loc.mod_path) - ) - use_package_path = os.path.dirname(os.path.abspath(node.loc.mod_path)) - else: - decl_package_path = use_package_path = "" - - if ( - node_info - and node.sym - and node_info.access == SymbolAccess.PROTECTED - and decl_package_path != use_package_path - ): - return self.error( - f'Can not access protected variable "{node.sym_name}" from {decl_package_path}' - f" to {use_package_path}." - ) - - if ( - node_info - and node.sym - and node_info.access == SymbolAccess.PRIVATE - and node.sym.defn[-1].loc.mod_path != node.loc.mod_path - ): - return self.error( - f'Can not access private variable "{node.sym_name}" from {node.sym.defn[-1].loc.mod_path}' - f" to {node.loc.mod_path}." - ) - def access_register( self, node: ast.AstSymbolNode, acc_tag: Optional[SymbolAccess] = None ) -> None: @@ -170,12 +135,121 @@ def enter_name(self, node: ast.Name) -> None: pos_start: int, pos_end: int, """ - from jaclang.compiler.passes import Pass - - if isinstance(node.parent, ast.FuncCall): - self.access_check(node) + # TODO: Enums are not considered at the moment, I'll need to test and add them bellow. + + # If the current node is a global variable's name there is no access, it's just the declaration. + if Pass.find_parent_of_type(node, ast.GlobalVars) is not None: + return + + # Class name, and ability name are declarations and there is no access here as well. + if isinstance(node.name_of, (ast.Ability, ast.Architype)): + return + + # Note that currently we can only check for name + symbols, because expressions are not associated with the + # typeinfo thus they don't have a symbol. In the future the name nodes will become expression nodes. + if not isinstance(node.sym, Symbol): + # In an expression 'foo.bar', the name bar doesn't contains any symbols if 'foo' is a module. + # In that case we'll manually get the module and check the access. This is one hacky way + # and needs to be done properly in the future. + if not (isinstance(node.parent, ast.AtomTrailer) and node.parent.is_attr): + return + + access_obj = node.parent.target + if not isinstance(access_obj, ast.Name) or access_obj.sym is None: + return + + if access_obj.sym.sym_type == SymbolType.MODULE: + curr_module: Optional[ast.Module] = Pass.find_parent_of_type( + node=node, typ=ast.Module + ) + if curr_module is None: + return + accessed_module: Optional[ast.Module] = None + for mod_dep in curr_module.mod_deps.values(): + if mod_dep.name == access_obj.sym.sym_name: + accessed_module = mod_dep + break + else: + return + + symbol: Optional[Symbol] = accessed_module.sym_tab.lookup(node.value) + if symbol is None: + # TODO: This is a symantic error, assuming that a non + # existing member access was reported by some other + # semantic analysis pass, as it's not the responsibility + # of the current pass. + return + + # Assuming toplevel things (class, vars, ability) cannot be protected. + if symbol.access == SymbolAccess.PRIVATE: + self.report_error( + f"Error: Invalid access of private member of module '{accessed_module.name}'.", + node, + ) + + # Not sure what else (except for module.member) can have a name + # node without symbol, we're just returning here for now. + return + + # Public symbols are fine. + if node.sym.access == SymbolAccess.PUBLIC: + return + + # Note that from bellow the access is either private or protected. + is_portect = node.sym.access == SymbolAccess.PROTECTED + access_type = "protected" if is_portect else "private" + + # Check if private member variable / ability is accessed outside of the class. + if node.sym.sym_type in (SymbolType.HAS_VAR, SymbolType.ABILITY): + + curr_class: Optional[ast.Architype] = Pass.find_parent_of_type( + node=node, typ=ast.Architype + ) + member_type: str = ( + "variable" if node.sym.sym_type == SymbolType.HAS_VAR else "ability" + ) - if node.sym and Pass.find_parent_of_type( - node=node.sym.defn[-1], typ=ast.GlobalVars - ): - self.access_check(node) + # Accessing a private member outside of a class. + if curr_class is None: + return self.report_error( + f"Error: Invalid access of {access_type} member {member_type} variable.", + node, + ) + + # Accessing a private member variable from a different or even inherited class. + assert isinstance(node.sym.parent_tab.owner, ast.Architype) + if curr_class != node.sym.parent_tab.owner: + if is_portect: + + # NOTE: This method is a hacky way to detect if the drivied class is inherit from base class, it + # doesn't work if the base class was provided as an expression (ex. obj Dri :module.Base: {...}). + # TODO: This function may be exists somewhere else (I cannot find) or else moved to somewhere + # common. + def is_class_inherited_from( + dri_class: ast.Architype, base_class: ast.Architype + ) -> bool: + if dri_class.base_classes is None: + return False + for expr in dri_class.base_classes.items: + if not isinstance(expr, ast.Name): + continue + if not isinstance(expr.name_of, ast.Architype): + continue # Unlikely. + if expr.name_of == base_class: + return True + if is_class_inherited_from(expr.name_of, base_class): + return True + return False + + if not is_class_inherited_from( + curr_class, node.sym.parent_tab.owner + ): + return self.report_error( + f"Error: Invalid access of {access_type} member {member_type}.", + node, + ) + else: + return self.report_error( + f"Error: Invalid access of {access_type} member {member_type}.", + node, + ) diff --git a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py index e5b393e0b..e088d60e1 100644 --- a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py +++ b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py @@ -6,11 +6,11 @@ from __future__ import annotations -from typing import Callable, TypeVar +from typing import Callable, Optional, TypeVar import jaclang.compiler.absyntree as ast from jaclang.compiler.passes import Pass -from jaclang.compiler.symtable import SymbolTable +from jaclang.compiler.symtable import Symbol from jaclang.settings import settings from jaclang.utils.helpers import pascal_to_snake from jaclang.vendor.mypy.nodes import Node as VNode # bit of a hack @@ -446,14 +446,37 @@ def exit_assignment(self, node: ast.Assignment) -> None: if self_obj.type_sym_tab and isinstance(right_obj, ast.AstSymbolNode): self_obj.type_sym_tab.def_insert(right_obj) + # NOTE: Note sure why we're inferring the symbol here instead of the sym table build pass + # I afraid that moving this there might break something. + def lookup_sym_from_node(self, node: ast.AstNode, member: str) -> Optional[Symbol]: + """Recursively look for the symbol of a member from a given node.""" + if isinstance(node, ast.AstSymbolNode): + if node.type_sym_tab is None: + return None + return node.type_sym_tab.lookup(member) + + if isinstance(node, ast.AtomTrailer): + if node.is_attr: # .member access. + return self.lookup_sym_from_node(node.right, member) + elif isinstance(node.right, ast.IndexSlice): + # NOTE: if the 'node.target' is a variable of type list[T] the + # node.target.sym_type is "builtins.list[T]" string Not sure how + # to get the type_sym_tab of "T" from just the name itself. would + # be better if the symbols types are not just strings but references. + # For now I'll mark them as todos. + # TODO: + # case 1: expr[i] -> regular indexing. + # case 2: expr[i:j:k] -> returns a sublist. + # case 3: expr["str"] -> dictionary lookup. + pass + + return None + def exit_name(self, node: ast.Name) -> None: """Add new symbols in the symbol table in case of atom trailer.""" - if isinstance(node.parent, ast.AtomTrailer): - target_node = node.parent.target - if isinstance(target_node, ast.AstSymbolNode): - parent_symbol_table = target_node.type_sym_tab - if isinstance(parent_symbol_table, SymbolTable): - node.sym = parent_symbol_table.lookup(node.sym_name) + if isinstance(node.parent, ast.AtomTrailer) and node.parent.target is not node: + sym = self.lookup_sym_from_node(node.parent.target, node.sym_name) + node.sym = sym or node.sym # def exit_in_for_stmt(self, node: ast.InForStmt): # print(node.loc.mod_path, node.loc) diff --git a/jaclang/tests/test_language.py b/jaclang/tests/test_language.py index 10b3b1883..6204ab27d 100644 --- a/jaclang/tests/test_language.py +++ b/jaclang/tests/test_language.py @@ -745,6 +745,7 @@ def test_deep_py_load_imports(self) -> None: # we can get rid of this, isn't? def test_access_modifier(self) -> None: """Test for access tags working.""" + return # TODO; captured_output = io.StringIO() sys.stdout = captured_output cli.check( From c658886c37fe0e7036260dd7b113fd77ab65848f Mon Sep 17 00:00:00 2001 From: Thakee Nathees Date: Fri, 16 Aug 2024 16:18:04 +0530 Subject: [PATCH 2/6] test cases were added and minor codechanges --- .../passes/main/access_modifier_pass.py | 103 +++++++++--------- jaclang/tests/fixtures/access_checker.jac | 29 ++--- jaclang/tests/fixtures/access_modifier.jac | 96 +++++++++------- 3 files changed, 124 insertions(+), 104 deletions(-) diff --git a/jaclang/compiler/passes/main/access_modifier_pass.py b/jaclang/compiler/passes/main/access_modifier_pass.py index 3fc8d1cae..9699aeda2 100644 --- a/jaclang/compiler/passes/main/access_modifier_pass.py +++ b/jaclang/compiler/passes/main/access_modifier_pass.py @@ -15,6 +15,25 @@ class AccessCheckPass(Pass): """Jac Ast Access Check pass.""" + # NOTE: This method is a hacky way to detect if the drivied class is inherit from base class, it + # doesn't work if the base class was provided as an expression (ex. obj Dri :module.Base: {...}). + def is_class_inherited_from( + self, dri_class: ast.Architype, base_class: ast.Architype + ) -> bool: + """Return true if the dri_class inherited from base_class.""" + if dri_class.base_classes is None: + return False + for expr in dri_class.base_classes.items: + if not isinstance(expr, ast.Name): + continue + if not isinstance(expr.name_of, ast.Architype): + continue # Unlikely. + if expr.name_of == base_class: + return True + if self.is_class_inherited_from(expr.name_of, base_class): + return True + return False + def report_error(self, message: str, node: Optional[ast.AstNode] = None) -> None: """Report error message related to illegal access of attributes and objects.""" self.error(message, node) @@ -142,7 +161,15 @@ def enter_name(self, node: ast.Name) -> None: return # Class name, and ability name are declarations and there is no access here as well. - if isinstance(node.name_of, (ast.Ability, ast.Architype)): + if isinstance(node.name_of, (ast.Ability, ast.Architype, ast.Enum)): + return + + # Get the context to check the access. + curr_class: Optional[ast.Architype] = Pass.find_parent_of_type( + node, ast.Architype + ) + curr_module: Optional[ast.Module] = Pass.find_parent_of_type(node, ast.Module) + if curr_module is None: return # Note that currently we can only check for name + symbols, because expressions are not associated with the @@ -159,11 +186,6 @@ def enter_name(self, node: ast.Name) -> None: return if access_obj.sym.sym_type == SymbolType.MODULE: - curr_module: Optional[ast.Module] = Pass.find_parent_of_type( - node=node, typ=ast.Module - ) - if curr_module is None: - return accessed_module: Optional[ast.Module] = None for mod_dep in curr_module.mod_deps.values(): if mod_dep.name == access_obj.sym.sym_name: @@ -174,7 +196,7 @@ def enter_name(self, node: ast.Name) -> None: symbol: Optional[Symbol] = accessed_module.sym_tab.lookup(node.value) if symbol is None: - # TODO: This is a symantic error, assuming that a non + # NOTE: This is a symantic error, assuming that a non # existing member access was reported by some other # semantic analysis pass, as it's not the responsibility # of the current pass. @@ -191,6 +213,8 @@ def enter_name(self, node: ast.Name) -> None: # node without symbol, we're just returning here for now. return + # From here (bellow) the node has a valid symbol. + # Public symbols are fine. if node.sym.access == SymbolAccess.PUBLIC: return @@ -199,57 +223,36 @@ def enter_name(self, node: ast.Name) -> None: is_portect = node.sym.access == SymbolAccess.PROTECTED access_type = "protected" if is_portect else "private" - # Check if private member variable / ability is accessed outside of the class. - if node.sym.sym_type in (SymbolType.HAS_VAR, SymbolType.ABILITY): + # The class we're currently in (None if we're not inside any). + sym_owner: ast.AstNode = node.sym.parent_tab.owner - curr_class: Optional[ast.Architype] = Pass.find_parent_of_type( - node=node, typ=ast.Architype - ) - member_type: str = ( - "variable" if node.sym.sym_type == SymbolType.HAS_VAR else "ability" - ) + # If the symbol belongs to a class, we need to check if the access used properly + # within the class and in it's inherited classes. + if isinstance(sym_owner, ast.Architype): - # Accessing a private member outside of a class. + # Accessing a private/protected member within the top level scope illegal. if curr_class is None: return self.report_error( - f"Error: Invalid access of {access_type} member {member_type} variable.", + f"Error: Invalid access of {access_type} member.", node, ) - # Accessing a private member variable from a different or even inherited class. - assert isinstance(node.sym.parent_tab.owner, ast.Architype) if curr_class != node.sym.parent_tab.owner: - if is_portect: - - # NOTE: This method is a hacky way to detect if the drivied class is inherit from base class, it - # doesn't work if the base class was provided as an expression (ex. obj Dri :module.Base: {...}). - # TODO: This function may be exists somewhere else (I cannot find) or else moved to somewhere - # common. - def is_class_inherited_from( - dri_class: ast.Architype, base_class: ast.Architype - ) -> bool: - if dri_class.base_classes is None: - return False - for expr in dri_class.base_classes.items: - if not isinstance(expr, ast.Name): - continue - if not isinstance(expr.name_of, ast.Architype): - continue # Unlikely. - if expr.name_of == base_class: - return True - if is_class_inherited_from(expr.name_of, base_class): - return True - return False - - if not is_class_inherited_from( - curr_class, node.sym.parent_tab.owner - ): - return self.report_error( - f"Error: Invalid access of {access_type} member {member_type}.", - node, - ) - else: + if not is_portect: # private member accessed in a different class. return self.report_error( - f"Error: Invalid access of {access_type} member {member_type}.", + f"Error: Invalid access of {access_type} member.", node, ) + else: # Accessing a protected member, check we're in an inherited class. + if not self.is_class_inherited_from(curr_class, sym_owner): + return self.report_error( + f"Error: Invalid access of {access_type} member.", + node, + ) + + elif isinstance(sym_owner, ast.Module) and sym_owner != curr_module: + # Accessing a private/public member in a different module. + return self.report_error( + f"Error: Invalid access of {access_type} member.", + node, + ) diff --git a/jaclang/tests/fixtures/access_checker.jac b/jaclang/tests/fixtures/access_checker.jac index 10f20aed5..e8041abb1 100644 --- a/jaclang/tests/fixtures/access_checker.jac +++ b/jaclang/tests/fixtures/access_checker.jac @@ -1,19 +1,14 @@ -obj :priv BankAccount { - has account_no: int; - can register() { - print(f"{self.account_no} has been registered."); - } -} -can :priv privmethod() { - return "private method"; -} -can :priv overide_check() { - return "inside access_check"; -} -glob :priv b=1; -glob :priv p=0; + +obj :pub ModulePublObj {} +obj :priv ModulePrivObj {} + +glob :pub module_publ_glob:int = 0; +glob :priv module_priv_glob:int = 0; + with entry { - public = BankAccount(123); - public.register(); - privmethod(); + ModulePrivObj(); # <-- okey. + ModulePublObj(); # <-- okey. + + module_publ_glob; # <-- okey. + module_priv_glob; # <-- okey. } diff --git a/jaclang/tests/fixtures/access_modifier.jac b/jaclang/tests/fixtures/access_modifier.jac index c897d772c..7fcf5a87c 100644 --- a/jaclang/tests/fixtures/access_modifier.jac +++ b/jaclang/tests/fixtures/access_modifier.jac @@ -1,49 +1,71 @@ include:jac access_checker; -obj Animal { - has :pub species: str; - can :priv introduce() { - return "Hello, I am " + self.species + ". "; - } +obj SomeObj { + obj :pub InnerPublObj {} + obj :priv InnerPrivObj {} + obj :protect InnerProtObj {} + enum :pub InnerPublEnum { FOO = "FOO" } + enum :priv InnerPrivEnum { BAR = "BAR" } + enum :protect InnerProtEnum { BAZ = "BAZ" } -} + can :pub publ_ability() -> None {} + can :priv priv_ability() -> None {} + can :protect prot_ability() -> None {} -obj Dog: Animal: { - has :pub breed: str; + has :pub publ_attrib:int = 0; + has :priv priv_attrib:int = 0; + has :protect prot_attrib:int = 0; +} - can init(breed: str) { - super.init("Dog"); - self.breed = breed; - } - can :pub bark() { - return "Woof! Woof!"; - } +obj :pub Chain1 { + has :priv priv_val:int = 0; # <-- the private in the chain. +} - can :pub makeSound() { - return "The dog says: " + super.introduce(); - } - obj Body { - has :pub s: str="22"; - } +obj :pub Chain2 { + has :pub chain1:Chain1 = Chain1(); } -can :priv overide_check() { - return "inside access_modifier"; + +obj :pub Chain3 { + has :pub chain2:Chain2 = Chain2(); } -glob :priv a=2, c=7, m="hi"; + + with entry { - myDog = Dog(breed= "Golden Retriever"); - - print( " Name: " + myDog.species ); - print( " Breed: " + myDog.breed ); - print( myDog.introduce() ); - print( myDog.bark() ); - b=myDog.Body("33"); - print(a); - account = BankAccount(3333); - print(privmethod()); - print(overide_check()); - print(b); - print(p); + SomeObj.InnerPublObj; # <-- okey. + SomeObj.InnerPrivObj; # <-- not okey. + SomeObj.InnerProtObj; # <-- not okey. + + SomeObj.InnerPublEnum; # <-- okey. + SomeObj.InnerPrivEnum; # <-- not okey. + SomeObj.InnerProtEnum; # <-- not okey. + + some_obj = SomeObj(); + + some_obj.InnerPublObj; # <-- okey. + some_obj.InnerPrivObj; # <-- not okey. + some_obj.InnerProtObj; # <-- not okey. + + some_obj.InnerPublEnum; # <-- okey. + some_obj.InnerPrivEnum; # <-- not okey. + some_obj.InnerProtEnum; # <-- not okey. + + some_obj.publ_attrib; # <-- okey. + some_obj.priv_attrib; # <-- not okey. + some_obj.prot_attrib; # <-- not okey. + + some_obj.publ_ability(); # <-- okey. + some_obj.priv_ability(); # <-- not okey. + some_obj.prot_ability(); # <-- not okey. + + chain3: Chain3 = Chain3(); + chain3.chain2.chain1; # <-- okey. + chain3.chain2.chain1.priv_val; # <-- not okey. + + access_checker.ModulePublObj(); # <-- okey. + access_checker.ModulePrivObj(); # <-- not okey. + + access_checker.module_priv_glob; # <-- okey. + access_checker.module_priv_glob; # <-- not okey. } From 490a19717082d5b6fc29193e3b8356202772ff25 Mon Sep 17 00:00:00 2001 From: mgtm98 Date: Fri, 16 Aug 2024 15:22:33 +0300 Subject: [PATCH 3/6] Adding a fix to get correct type symbol table in case of ModuleType --- .../passes/main/access_modifier_pass.py | 40 +------------------ .../passes/main/fuse_typeinfo_pass.py | 3 ++ 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/jaclang/compiler/passes/main/access_modifier_pass.py b/jaclang/compiler/passes/main/access_modifier_pass.py index 7d4dc2ffa..0fc375edb 100644 --- a/jaclang/compiler/passes/main/access_modifier_pass.py +++ b/jaclang/compiler/passes/main/access_modifier_pass.py @@ -6,7 +6,7 @@ from typing import Optional import jaclang.compiler.absyntree as ast -from jaclang.compiler.constant import SymbolAccess, SymbolType +from jaclang.compiler.constant import SymbolAccess from jaclang.compiler.passes import Pass from jaclang.compiler.symtable import Symbol from jaclang.settings import settings @@ -185,46 +185,8 @@ def enter_name(self, node: ast.Name) -> None: # Note that currently we can only check for name + symbols, because expressions are not associated with the # typeinfo thus they don't have a symbol. In the future the name nodes will become expression nodes. if not isinstance(node.sym, Symbol): - # In an expression 'foo.bar', the name bar doesn't contains any symbols if 'foo' is a module. - # In that case we'll manually get the module and check the access. This is one hacky way - # and needs to be done properly in the future. - if not (isinstance(node.parent, ast.AtomTrailer) and node.parent.is_attr): - return - - access_obj = node.parent.target - if not isinstance(access_obj, ast.Name) or access_obj.sym is None: - return - - if access_obj.sym.sym_type == SymbolType.MODULE: - accessed_module: Optional[ast.Module] = None - for mod_dep in curr_module.mod_deps.values(): - if mod_dep.name == access_obj.sym.sym_name: - accessed_module = mod_dep - break - else: - return - - symbol: Optional[Symbol] = accessed_module.sym_tab.lookup(node.value) - if symbol is None: - # NOTE: This is a symantic error, assuming that a non - # existing member access was reported by some other - # semantic analysis pass, as it's not the responsibility - # of the current pass. - return - - # Assuming toplevel things (class, vars, ability) cannot be protected. - if symbol.access == SymbolAccess.PRIVATE: - self.report_error( - f"Error: Invalid access of private member of module '{accessed_module.name}'.", - node, - ) - - # Not sure what else (except for module.member) can have a name - # node without symbol, we're just returning here for now. return - # From here (bellow) the node has a valid symbol. - # Public symbols are fine. if node.sym.access == SymbolAccess.PUBLIC: return diff --git a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py index a4186ded3..3c2e8cab8 100644 --- a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py +++ b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py @@ -52,6 +52,9 @@ def __set_sym_table_link(self, node: ast.AstSymbolNode) -> None: if typ[0] == "builtins": return + if node.sym_type == "types.ModuleType" and node.sym: + node.name_spec.type_sym_tab = node.sym.parent_tab.find_scope(node.sym_name) + assert isinstance(self.ir, ast.Module) if typ_sym_table: From 63d244916e620151eb8843319a55201295fdffbe Mon Sep 17 00:00:00 2001 From: mgtm98 Date: Fri, 16 Aug 2024 15:48:15 +0300 Subject: [PATCH 4/6] Erroring out with the method/var name that has the issue --- .../passes/main/access_modifier_pass.py | 8 +-- .../passes/main/fuse_typeinfo_pass.py | 54 +++++++++---------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/jaclang/compiler/passes/main/access_modifier_pass.py b/jaclang/compiler/passes/main/access_modifier_pass.py index 0fc375edb..7e897865b 100644 --- a/jaclang/compiler/passes/main/access_modifier_pass.py +++ b/jaclang/compiler/passes/main/access_modifier_pass.py @@ -205,26 +205,26 @@ def enter_name(self, node: ast.Name) -> None: # Accessing a private/protected member within the top level scope illegal. if curr_class is None: return self.report_error( - f"Error: Invalid access of {access_type} member.", + f'Error: Invalid access of {access_type} member "{node.sym_name}".', node, ) if curr_class != node.sym.parent_tab.owner: if not is_portect: # private member accessed in a different class. return self.report_error( - f"Error: Invalid access of {access_type} member.", + f'Error: Invalid access of {access_type} member "{node.sym_name}".', node, ) else: # Accessing a protected member, check we're in an inherited class. if not self.is_class_inherited_from(curr_class, sym_owner): return self.report_error( - f"Error: Invalid access of {access_type} member.", + f'Error: Invalid access of {access_type} member "{node.sym_name}".', node, ) elif isinstance(sym_owner, ast.Module) and sym_owner != curr_module: # Accessing a private/public member in a different module. return self.report_error( - f"Error: Invalid access of {access_type} member.", + f'Error: Invalid access of {access_type} member "{node.sym_name}".', node, ) diff --git a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py index 3c2e8cab8..dc9e4c381 100644 --- a/jaclang/compiler/passes/main/fuse_typeinfo_pass.py +++ b/jaclang/compiler/passes/main/fuse_typeinfo_pass.py @@ -6,11 +6,10 @@ from __future__ import annotations -from typing import Callable, Optional, TypeVar +from typing import Callable, TypeVar import jaclang.compiler.absyntree as ast from jaclang.compiler.passes import Pass -from jaclang.compiler.symtable import Symbol from jaclang.settings import settings from jaclang.utils.helpers import pascal_to_snake from jaclang.vendor.mypy.nodes import Node as VNode # bit of a hack @@ -494,28 +493,29 @@ def exit_atom_trailer(self, node: ast.AtomTrailer) -> None: ): # TODO check why IndexSlice produce an issue right.name_spec.sym = left.type_sym_tab.lookup(right.sym_name) - # NOTE: Note sure why we're inferring the symbol here instead of the sym table build pass - # I afraid that moving this there might break something. - def lookup_sym_from_node(self, node: ast.AstNode, member: str) -> Optional[Symbol]: - """Recursively look for the symbol of a member from a given node.""" - if isinstance(node, ast.AstSymbolNode): - if node.type_sym_tab is None: - return None - return node.type_sym_tab.lookup(member) - - if isinstance(node, ast.AtomTrailer): - if node.is_attr: # .member access. - return self.lookup_sym_from_node(node.right, member) - elif isinstance(node.right, ast.IndexSlice): - # NOTE: if the 'node.target' is a variable of type list[T] the - # node.target.sym_type is "builtins.list[T]" string Not sure how - # to get the type_sym_tab of "T" from just the name itself. would - # be better if the symbols types are not just strings but references. - # For now I'll mark them as todos. - # TODO: - # case 1: expr[i] -> regular indexing. - # case 2: expr[i:j:k] -> returns a sublist. - # case 3: expr["str"] -> dictionary lookup. - pass - - return None + # # TODO [Gamal]: Need to support getting the type of an expression + # # NOTE: Note sure why we're inferring the symbol here instead of the sym table build pass + # # I afraid that moving this there might break something. + # def lookup_sym_from_node(self, node: ast.AstNode, member: str) -> Optional[Symbol]: + # """Recursively look for the symbol of a member from a given node.""" + # if isinstance(node, ast.AstSymbolNode): + # if node.type_sym_tab is None: + # return None + # return node.type_sym_tab.lookup(member) + + # if isinstance(node, ast.AtomTrailer): + # if node.is_attr: # .member access. + # return self.lookup_sym_from_node(node.right, member) + # elif isinstance(node.right, ast.IndexSlice): + # # NOTE: if the 'node.target' is a variable of type list[T] the + # # node.target.sym_type is "builtins.list[T]" string Not sure how + # # to get the type_sym_tab of "T" from just the name itself. would + # # be better if the symbols types are not just strings but references. + # # For now I'll mark them as todos. + # # TODO: + # # case 1: expr[i] -> regular indexing. + # # case 2: expr[i:j:k] -> returns a sublist. + # # case 3: expr["str"] -> dictionary lookup. + # pass + + # return None From 2a0eed964c1d35209c4496a8c7d07241d228bbe0 Mon Sep 17 00:00:00 2001 From: Thakee Nathees Date: Sat, 17 Aug 2024 23:54:16 +0530 Subject: [PATCH 5/6] test cases were added --- jaclang/tests/fixtures/access_modifier.jac | 33 ++++++++++++++++++++++ jaclang/tests/test_language.py | 6 +--- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/jaclang/tests/fixtures/access_modifier.jac b/jaclang/tests/fixtures/access_modifier.jac index 7fcf5a87c..1b019d871 100644 --- a/jaclang/tests/fixtures/access_modifier.jac +++ b/jaclang/tests/fixtures/access_modifier.jac @@ -19,6 +19,39 @@ obj SomeObj { } +obj BaseClass { + has :pub publ_attrib:int = 0; + has :priv priv_attrib:int = 0; + has :protect prot_attrib:int = 0; + + can do_base() -> None { + self.publ_attrib; # <-- okey. + self.priv_attrib; # <-- okey. + self.prot_attrib; # <-- okey. + } +} + + +obj DrivedClass1 :BaseClass: { + can do_driv1() -> None { + self.publ_attrib; # <-- okey. + self.priv_attrib; # <-- not okey. + self.prot_attrib; # <-- okey. + } +} + + +# Check if the protect works from inherited chain. +obj DrivedClass2 :BaseClass: { + can do_driv2() -> None { + self.publ_attrib; # <-- okey. + self.priv_attrib; # <-- not okey. + self.prot_attrib; # <-- okey. + } +} + + + obj :pub Chain1 { has :priv priv_val:int = 0; # <-- the private in the chain. } diff --git a/jaclang/tests/test_language.py b/jaclang/tests/test_language.py index 6204ab27d..4f733c67b 100644 --- a/jaclang/tests/test_language.py +++ b/jaclang/tests/test_language.py @@ -745,7 +745,6 @@ def test_deep_py_load_imports(self) -> None: # we can get rid of this, isn't? def test_access_modifier(self) -> None: """Test for access tags working.""" - return # TODO; captured_output = io.StringIO() sys.stdout = captured_output cli.check( @@ -754,10 +753,7 @@ def test_access_modifier(self) -> None: ) sys.stdout = sys.__stdout__ stdout_value = captured_output.getvalue() - self.assertIn('Can not access private variable "p"', stdout_value) - self.assertIn('Can not access private variable "privmethod"', stdout_value) - self.assertIn('Can not access private variable "BankAccount"', stdout_value) - self.assertNotIn(" Name: ", stdout_value) + self.assertEqual(stdout_value.count("Invalid access"), 15) def test_deep_convert(self) -> None: """Test py ast to Jac ast conversion output.""" From 6daf24aad5f422ef18297a677f80b92c624bbe11 Mon Sep 17 00:00:00 2001 From: marsninja Date: Thu, 29 Aug 2024 09:10:02 -0400 Subject: [PATCH 6/6] refactor: clean up dead code --- .../passes/main/access_modifier_pass.py | 100 ------------------ 1 file changed, 100 deletions(-) diff --git a/jaclang/compiler/passes/main/access_modifier_pass.py b/jaclang/compiler/passes/main/access_modifier_pass.py index 7e897865b..d3a30a184 100644 --- a/jaclang/compiler/passes/main/access_modifier_pass.py +++ b/jaclang/compiler/passes/main/access_modifier_pass.py @@ -38,10 +38,6 @@ def report_error(self, message: str, node: Optional[ast.AstNode] = None) -> None """Report error message related to illegal access of attributes and objects.""" self.error(message, node) - def after_pass(self) -> None: - """After pass.""" - pass - def exit_node(self, node: ast.AstNode) -> None: # TODO: Move to debug pass """Exit node.""" super().exit_node(node) @@ -58,102 +54,6 @@ def exit_node(self, node: ast.AstNode) -> None: # TODO: Move to debug pass ): self.warning(f"Name {node.sym_name} not present in symbol table") - def access_register( - self, node: ast.AstSymbolNode, acc_tag: Optional[SymbolAccess] = None - ) -> None: - """Access register.""" - - def enter_global_vars(self, node: ast.GlobalVars) -> None: - """Sub objects. - - access: Optional[SubTag[Token]], - assignments: SubNodeList[Assignment], - is_frozen: bool, - """ - pass - - def enter_module(self, node: ast.Module) -> None: - """Sub objects. - - name: str, - doc: Token, - body: Optional['Elements'], - mod_path: str, - is_imported: bool, - """ - - def enter_architype(self, node: ast.Architype) -> None: - """Sub objects. - - name: Name, - arch_type: Token, - access: Optional[SubTag[Token]], - base_classes: Optional[SubNodeList[Expr]], - body: Optional[SubNodeList[ArchBlockStmt] | ArchDef], - decorators: Optional[SubNodeList[Expr]] = None, - """ - pass - - def enter_enum(self, node: ast.Enum) -> None: - """Sub objects. - - name: Name, - access: Optional[SubTag[Token]], - base_classes: Optional[SubNodeList[Expr]], - body: Optional[SubNodeList[EnumBlockStmt] | EnumDef], - decorators: Optional[SubNodeList[Expr]] = None, - """ - pass - - def enter_ability(self, node: ast.Ability) -> None: - """Sub objects. - - name_ref: NameSpec, - is_func: bool, - is_async: bool, - is_override: bool, - is_static: bool, - is_abstract: bool, - access: Optional[SubTag[Token]], - signature: Optional[FuncSignature | EventSignature], - body: Optional[SubNodeList[CodeBlockStmt] | AbilityDef], - decorators: Optional[SubNodeList[Expr]] = None, - """ - pass - - def enter_sub_node_list(self, node: ast.SubNodeList) -> None: - """Sub objects. - - items: list[T] - """ - - def enter_arch_has(self, node: ast.ArchHas) -> None: - """Sub objects. - - is_static: bool, - access: Optional[SubTag[Token]], - vars: SubNodeList[HasVar], - is_frozen: bool, - """ - pass - - def enter_atom_trailer(self, node: ast.AtomTrailer) -> None: - """Sub objects. - - access: Optional[SubTag[Token]], - """ - pass - - def enter_func_call(self, node: ast.FuncCall) -> None: - """Sub objects. - - target: Expr, - params: Optional[SubNodeList[Expr | KWPair]], - genai_call: Optional[FuncCall], - kid: Sequence[AstNode], - """ - pass - def enter_name(self, node: ast.Name) -> None: """Sub objects.