Feature/linter

Pipfile

@@ -5,6 +5,8 @@ name = "pypi"
 
 [packages]
 coverage = "==5.0a1"
+pylint = "*"
+git-pylint-commit-hook = "*"
 
 [dev-packages]
 

contextshell/action.py

@@ -43,7 +43,13 @@ def action_from_function(function_to_wrap: Callable) -> Action:
     return CallableAction(function_to_wrap, action_path)
 
 
-class ActionExecutor:
+class Executor(ABC):
+    @abstractmethod
+    def execute(self, target: NodePath, action_name: NodePath, args: ActionArgsPack = None):
+        raise NotImplementedError()
+
+
+class ActionExecutor(Executor):
     """Interface for backends allowing execution of arbitrary actions"""
 
     def find_action(self, target: NodePath, action: NodePath) -> Optional[Action]:
@@ -66,7 +72,7 @@ def unpack_argument_tree(action_args: ActionArgsPack) -> Tuple[PositionalArgumen
             args[key] = value
         else:
             kwargs[key.to_python_name()] = value
-    assert len(args) == 0 or max(args.keys()) < len(args)+len(kwargs)
+    assert not args or max(args.keys()) < len(args)+len(kwargs)
     positional_args = [a[1] for a in sorted(args.items())]
     return positional_args, kwargs
 

contextshell/backends/FilesystemTree.py

@@ -17,7 +17,7 @@ def __init__(self, root_directory_path: str):
         self.register_builtin_action(action_from_function(self.list_actions_action))
 
     def _to_os_path(self, *paths: NodePath) -> Path:
-        node_paths = map(lambda p: NodePath(p), paths)
+        node_paths = map(NodePath, paths)
         os_paths = map(lambda p: Path('').joinpath(*p), node_paths)
         return self.root_directory_path.joinpath(*os_paths)
 

contextshell/backends/NodeTree.py

@@ -22,18 +22,20 @@ def get(self):
 
     def set(self, new_value):
         """Store provided value in this node"""
-        if type(self._value) != type(new_value):
-            raise TypeError("Cannot assign value with type '{}' to '{}' node".format(type(new_value).__name__,
-                                                                                     type(self._value).__name__))
+        if not isinstance(new_value, type(self._value)):
+            new_type = type(new_value).__name__
+            current_type = type(self._value).__name__
+            raise TypeError(
+                f"Cannot assign value with type '{new_type}' to '{current_type}' node")
         self._value = new_value
 
     def list(self):
         """List names of the subnodes"""
         names = map(lambda p: p[0], self._subnodes)
         index = 0
         indexed_names = []
-        for n in names:
-            indexed_names.append(n if n is not None else index)
+        for name in names:
+            indexed_names.append(name if name is not None else index)
             index += 1
         return indexed_names
 
@@ -42,7 +44,7 @@ def append(self, node, name: str = None):
         if node is None:
             raise ValueError("Cannot append None as node")
         if name is not None:
-            if len(name) == 0:
+            if not name:
                 raise NameError("Invalid appended node name - empty")
             if self.get_node(name) is not None:
                 raise NameError("Node '{}' already exists".format(name))
@@ -52,9 +54,9 @@ def append(self, node, name: str = None):
     def get_node(self, name: str = None, index: int = None) -> Optional['Node']:
         """Return subnode with provided name or index"""
         if name is not None:
-            for p in self._subnodes:
-                if p[0] == name:
-                    return p[1]
+            for name_node_pair in self._subnodes:
+                if name_node_pair[0] == name:
+                    return name_node_pair[1]
         elif index is not None:
             if 0 <= index < len(self._subnodes):
                 return self._subnodes[index][1]
@@ -153,7 +155,7 @@ def install_action(self, target: NodePath, action: Action):
         self.create(NodePath.join(target, '@actions', action.name), action)
 
     def find_first_in(self, candidate_paths: List[NodePath]) -> Optional[Node]:
-        if candidate_paths is None or len(candidate_paths) == 0:
+        if candidate_paths is None or not candidate_paths:
             raise ValueError("No candidate paths provided")
         for path in candidate_paths:
             node = self._resolve_optional_path(path)
@@ -177,10 +179,9 @@ def list_actions(self, path: NodePath) -> List[NodePath]:
         action_paths = self.list(NodePath.join(path, '@actions'))
         action_paths = sorted(action_paths)
 
-        if len(path) == 0:  # Root
+        if not path:  # Root
             return action_paths
-        else:
-            return action_paths + self.list_actions(path.base_path)
+        return action_paths + self.list_actions(path.base_path)
 
     def install_global_type(self, node_type):
         self.install_type(NodePath('.'), node_type)
@@ -240,7 +241,7 @@ def _resolve_optional_path(self, path: NodePath, root: Node = None) -> Optional[
                 raise ValueError("Could not resolve relative paths")
             root = self.root
             assert root is not None
-        if len(path) == 0:
+        if not path:
             return root
 
         next_branch_name = path[0]
@@ -254,7 +255,7 @@ def _create_path(self, path: NodePath, root: Node = None) -> Node:
                 raise ValueError("Could not resolve relative paths")
             root = self.root
             assert root is not None
-        if len(path) == 0:
+        if not path:
             return root
 
         next_branch_name = path[0]

contextshell/backends/VirtualTree.py

@@ -1,17 +1,18 @@
 from collections import OrderedDict
+from typing import Dict
 
-from ..action import ActionExecutor, ActionArgsPack
+from ..action import Executor, ActionArgsPack
 from ..path import NodePath
 
 
-class VirtualTree(ActionExecutor):
+class VirtualTree(Executor):
     """Abstract frontend allowing embedding (mounting) of more specific tree roots"""
 
     def __init__(self):
-        self.mounts: OrderedDict[NodePath, ActionExecutor] = OrderedDict()
+        self.mounts: Dict[NodePath, Executor] = OrderedDict()
 
     # TODO: rename to attach/detach
-    def mount(self, path: NodePath, root: ActionExecutor):
+    def mount(self, path: NodePath, root: Executor):
         if path.is_relative:
             raise ValueError("Could not mount relative path")
         if path in self.mounts:
@@ -27,7 +28,7 @@ def path_length_extractor(path_root_pair):
     def umount(self, path: NodePath):
         del self.mounts[path]
 
-    def execute(self, target: NodePath, action: NodePath, args: ActionArgsPack = None):
+    def execute(self, target: NodePath, action_name: NodePath, args: ActionArgsPack = None):
         if target.is_relative:
             raise ValueError("Could not invoke action with relative target path")
         if not args:
@@ -36,6 +37,5 @@ def execute(self, target: NodePath, action: NodePath, args: ActionArgsPack = Non
             if path.is_parent_of(target):
                 remapped_target = target.relative_to(path)
                 remapped_target.is_absolute = True
-                return root.execute(remapped_target, action, args)
-        else:
-            raise RuntimeError("Could not find provider for path: '{}'".format(target))
+                return root.execute(remapped_target, action_name, args)
+        raise RuntimeError("Could not find provider for path: '{}'".format(target))

contextshell/command.py

@@ -27,6 +27,7 @@ def _to_string(param):
 
 
 class CommandInterpreter:
+    """Actually executes commands on provided backend tree"""
     def __init__(self, tree: ActionExecutor) -> None:
         self.tree = tree
 
@@ -66,7 +67,7 @@ def tokenize(text: str) -> List[str]:
 
     def finish_token():
         nonlocal tok, tokens
-        if len(tok) > 0:
+        if tok:
             tok = convert_token_type(tok)
             tokens.append(tok)
             tok = ''
@@ -103,7 +104,7 @@ def __init__(self):
 
     def parse(self, command_line: str) -> Optional[Command]:
         tokens = tokenize(command_line)
-        if len(tokens) == 0:
+        if not tokens:
             return None  # ignore empty lines
         if tokens[0] == '#':
             return None  # ignore comments
@@ -116,17 +117,16 @@ def _parse_scope(self, token_iterator) -> Command:
         for tok in token_iterator:
             if tok == '{':
                 parts.append(self._parse_scope(token_iterator))
-            elif tok == '}':
-                break
-            else:
+            elif tok != '}':
                 parts.append(tok)
+            else:
+                break
 
         if ':' in parts:
             cmd = Command(parts[2])
             cmd.target = parts[0]
             cmd.arguments = parts[3:]
-            return cmd
         else:
             cmd = Command(parts[0])
             cmd.arguments = parts[1:]
-            return cmd
+        return cmd

contextshell/path.py

@@ -30,7 +30,7 @@ def to_python_name(self) -> str:
         name = name.lstrip(NodePath.separator).replace(NodePath.separator, '_')
         return name
 
-    def __init__(self, representation=[], absolute=False):
+    def __init__(self, representation=None, absolute=False):
         super().__init__()
         self.is_absolute = absolute
         if isinstance(representation, int):
@@ -40,10 +40,12 @@ def __init__(self, representation=[], absolute=False):
         elif isinstance(representation, NodePath):
             self.is_absolute = representation.is_absolute
             self.extend(representation)
-        elif isinstance(representation, list) or isinstance(representation, tuple):
+        elif isinstance(representation, (list, tuple)):
             # TODO: check element's types?
             # TODO: is it ever used?
             self.extend(representation)
+        elif representation is None:
+            pass
         else:
             raise ValueError("Could not convert {} to NodePath".format(representation))
 
@@ -65,21 +67,21 @@ def base_path(self):
     @property
     def base_name(self):
         """Returns last path element"""
-        if len(self) == 0:
+        if not self:
             return None
         return self[-1]
 
     def is_parent_of(self, other: 'NodePath') -> bool:
         """Checks if provided path prefix matches self"""
         other = NodePath.cast(other)
         if self.is_absolute != other.is_absolute:
-            raise ValueError("Cannot compare absolute and relative paths: {} and {}".format(self, other))
+            raise ValueError(f"Cannot compare absolute and relative paths: {self} and {other}")
         return NodePath(other[:len(self)], absolute=other.is_absolute) == self
 
     def relative_to(self, other) -> 'NodePath':
         """Make current path relative to provided one by removing common prefix"""
         if not other.is_parent_of(self):
-            raise ValueError("{} is not relative to {}".format(self, other))
+            raise ValueError(f"{self} is not relative to {other}")
         return NodePath(self[len(other):], absolute=False)
 
     @staticmethod
@@ -94,15 +96,16 @@ def _parse_path(self, text):
         text = text.strip()
         if text.startswith(NodePath.separator):
             self.is_absolute = True
-        new_path = map(NodePath._to_path_part, [part for part in text.split(NodePath.separator) if len(part) > 0])
+        non_empty_path_parts = [part for part in text.split(NodePath.separator) if part]
+        new_path = map(NodePath._to_path_part, non_empty_path_parts)
         self.extend(new_path)
 
     def __eq__(self, other):
         other = NodePath.cast(other)
         return self.is_absolute == other.is_absolute and self[:] == other[:]
 
     def __ne__(self, other):
-        return not (self == other)
+        return not self == other
 
     def __str__(self):
         text_representation = NodePath.separator.join(map(str, self))