Repository: VirtusLab/git-machete (python)

Issue:
`AttributeError: 'str' object has no attribute 'full_name'` due to (?) mistyped `CommandLineOptions.opt_branch`
For an already existing, but not yet managed branch `foo/bar`:

```
$ git machete add foo/bar
Traceback (most recent call last):
  File "/usr/local/bin/git-machete", line 4, in <module>
    __import__('pkg_resources').run_script('git-machete==3.5.0', 'git-machete')
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 666, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/usr/lib/python3.7/site-packages/pkg_resources/__init__.py", line 1469, in run_script
    exec(script_code, namespace, namespace)
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/EGG-INFO/scripts/git-machete", line 19, in <module>
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/cli.py", line 803, in main
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/cli.py", line 522, in launch
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/client.py", line 263, in add
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/client.py", line 1502, in __infer_upstream
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/client.py", line 1479, in __match_log_to_filtered_reflogs
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/git_operations.py", line 495, in spoonfeed_log_shas
  File "/usr/local/lib/python3.7/site-packages/git_machete-3.5.0-py3.7.egg/git_machete/git_operations.py", line 487, in __get_log_shas
AttributeError: 'str' object has no attribute 'full_name'
```

This is most likely due to assignment `cli_opts.opt_branch = arg`, with `arg` being `str`-typed (?)... not sure why it passed mypy in the first place, pls investigate.

Also, regression test + preferable a systematic solution against this kind of errors would be very useful.

Pull request:
Issue #328: Bugfix - mistyped CommandLineOptions.opt_branch
closes #328 

Mypy didn't catch obvious bug when function typed to return `LocalBranchShortName` actually returns a string instead.
I have manually checked functions which return something for type their type correctness, but here seems to be the only occurence.

Regression test for  `git machete add` is also included.

Patch:
diff --git a/git_machete/cli.py b/git_machete/cli.py
index 19a6a38..8824ec9 100644
--- a/git_machete/cli.py
+++ b/git_machete/cli.py
@@ -358,15 +358,25 @@ def create_cli_parser() -> argparse.ArgumentParser:
 def update_cli_opts_using_parsed_args(
         cli_opts: git_machete.options.CommandLineOptions,
         parsed_args: argparse.Namespace) -> None:
+
+    # Warning: In mypy, Arguments that come from untyped functions/variables are silently treated by mypy as Any.
+    # Since argparse is not typed, everything that comes from argparse.Namespace will be taken as Any :(
+    # Even if we add type=LocalBranchShortName into argument parser for branch,
+    # python debugger will see branch as LocalBranchShortName but mypy always will see it as Any,
+    # until you specifically tell mypy what is the exact type by casting (right now it's done this way below, but casting does not solve all of the problems).
+    #
+    # The reasonable solution here would be to use Typed Argument Parser which is a wrapper over argparse with modernised solution for typing.
+    # But it would add external dependency to git-machete, so let's stick to current casting.
+
     for opt, arg in vars(parsed_args).items():
         if opt == "branch":
-            cli_opts.opt_branch = arg
+            cli_opts.opt_branch = LocalBranchShortName.of(arg)
         elif opt == "checked_out_since":
             cli_opts.opt_checked_out_since = arg
         elif opt == "color":
             cli_opts.opt_color = arg
         elif opt == "down_fork_point":
-            cli_opts.opt_down_fork_point = arg
+            cli_opts.opt_down_fork_point = AnyRevision.of(arg)
         elif opt == "debug":
             cli_opts.opt_debug = True
         elif opt == "draft":
@@ -374,7 +384,7 @@ def update_cli_opts_using_parsed_args(
         elif opt == "fetch":
             cli_opts.opt_fetch = True
         elif opt == "fork_point":
-            cli_opts.opt_fork_point = arg
+            cli_opts.opt_fork_point = AnyRevision.of(arg)
         elif opt == "inferred":
             cli_opts.opt_inferred = True
         elif opt == "list_commits_with_hashes":
@@ -397,7 +407,7 @@ def update_cli_opts_using_parsed_args(
         elif opt == "no_push_untracked":
             cli_opts.opt_push_untracked = False
         elif opt == "onto":
-            cli_opts.opt_onto = arg
+            cli_opts.opt_onto = LocalBranchShortName.of(arg)
         elif opt == "override_to":
             cli_opts.opt_override_to = arg
         elif opt == "override_to_inferred":
@@ -412,7 +422,7 @@ def update_cli_opts_using_parsed_args(
         elif opt == "as_root":
             cli_opts.opt_as_root = True
         elif opt == "roots":
-            cli_opts.opt_roots = arg.split(",")
+            cli_opts.opt_roots = list(map(LocalBranchShortName.of, arg.split(",")))
         elif opt == "return_to":
             cli_opts.opt_return_to = arg
         elif opt == "stat":


Test patch:
diff --git a/git_machete/tests/functional/test_machete.py b/git_machete/tests/functional/test_machete.py
index e6f934d..36165d7 100644
--- a/git_machete/tests/functional/test_machete.py
+++ b/git_machete/tests/functional/test_machete.py
@@ -2647,3 +2647,31 @@ class MacheteTester(unittest.TestCase):
             roots_only_commit_hash, log_content,
             msg="Verify that commits from parent branch are not visible when "
                 "executing `git machete log`.")
+
+    @mock.patch('git_machete.utils.run_cmd', mock_run_cmd)  # to hide git outputs in tests
+    def test_add(self) -> None:
+        """
+        Verify behaviour of a 'git machete add' command.
+        """
+        (
+            self.repo_sandbox.new_branch("master")
+                .commit("master commit.")
+                .new_branch("develop")
+                .commit("develop commit.")
+                .new_branch("feature")
+                .commit("feature commit.")
+                .check_out("develop")
+                .commit("New commit on develop")
+        )
+        self.launch_command("discover", "-y")
+        self.repo_sandbox.new_branch("bugfix/feature_fail")
+
+        self.assert_command(['add', '-y', 'bugfix/feature_fail'], 'Adding `bugfix/feature_fail` onto the inferred upstream (parent) branch `develop`\n'
+                                                                  'Added branch `bugfix/feature_fail` onto `develop`\n', strip_indentation=False)
+
+        # test with --onto option
+        self.repo_sandbox.new_branch("chore/remove_indentation")
+
+        self.assert_command(['add', '--onto=feature'],
+                            'Added branch `chore/remove_indentation` onto `feature`\n',
+                            strip_indentation=False)