From 6bb2256043e5d19dd2eb3e06e9ca30b4604fd374 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Sat, 21 Jul 2018 17:11:21 +0200
Subject: [PATCH] Die with a meaningful error if a deprecated command is used

In case a deprecated form of the module command is used, the program
will exit non-zero and print an informative error message suggesting
which command should be used instead.
---
 lib/spack/spack/cmd/module.py      | 48 ++++++++++++++++++++++++++++--
 lib/spack/spack/test/cmd/module.py | 12 ++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index 22826afdf3..ea54f3b691 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -22,6 +22,10 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
+import argparse
+
+import llnl.util.tty as tty
+
 import spack.cmd.modules.dotkit
 import spack.cmd.modules.lmod
 import spack.cmd.modules.tcl
@@ -33,13 +37,51 @@
 
 _subcommands = {}
 
+_deprecated_commands = ('refresh', 'find', 'rm', 'loads')
+
 
 def setup_parser(subparser):
-    sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_type')
+    sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command')
     spack.cmd.modules.dotkit.add_command(sp, _subcommands)
     spack.cmd.modules.lmod.add_command(sp, _subcommands)
     spack.cmd.modules.tcl.add_command(sp, _subcommands)
 
+    for name in _deprecated_commands:
+        add_deprecated_command(sp, name)
 
-def module(parser, args):
-    _subcommands[args.module_type](parser, args)
+
+def add_deprecated_command(subparser, name):
+    parser = subparser.add_parser(name)
+    parser.add_argument(
+        '-m', '--module-type', help=argparse.SUPPRESS,
+        choices=spack.modules.module_types.keys(), action='append'
+    )
+
+
+def handle_deprecated_command(args, unknown_args):
+    command = args.module_command
+    unknown = ' '.join(unknown_args)
+
+    module_types = args.module_type or 'tcl'
+
+    msg = '`spack module {0} {1} ...` has been moved. Try this instead:\n'
+    msg = msg.format(command, ' '.join('-m ' + x for x in module_types))
+    for x in module_types:
+        msg += '\n\t$ spack module {0} {1} {2}'.format(x, command, unknown)
+    msg += '\n'
+
+    tty.die(msg)
+
+
+def module(parser, args, unknown_args):
+
+    # Here we permit unknown arguments to intercept deprecated calls
+    if args.module_command in _deprecated_commands:
+        handle_deprecated_command(args, unknown_args)
+
+    # Fail if unknown arguments are present, once we excluded a deprecated
+    # command
+    if unknown_args:
+        tty.die('unrecognized arguments: {0}'.format(' '.join(unknown_args)))
+
+    _subcommands[args.module_command](parser, args)
diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py
index ca60814147..49e327115a 100644
--- a/lib/spack/spack/test/cmd/module.py
+++ b/lib/spack/spack/test/cmd/module.py
@@ -43,6 +43,7 @@ def _module_files(module_type, *specs):
         ['rm', 'doesnotexist'],  # Try to remove a non existing module
         ['find', 'mpileaks'],  # Try to find a module with multiple matches
         ['find', 'doesnotexist'],  # Try to find a module with no matches
+        ['find', '--unkown_args'],  # Try to give an unknown argument
     ]
 )
 def failure_args(request):
@@ -67,6 +68,17 @@ def test_exit_with_failure(database, module_type, failure_args):
         module(module_type, *failure_args)
 
 
+@pytest.mark.db
+@pytest.mark.parametrize('deprecated_command', [
+    ('refresh', '-m', 'tcl', 'mpileaks'),
+    ('rm', '-m', 'tcl', '-m', 'lmod', 'mpileaks'),
+    ('find', 'mpileaks'),
+])
+def test_deprecated_command(database, deprecated_command):
+    with pytest.raises(spack.main.SpackCommandError):
+        module(*deprecated_command)
+
+
 @pytest.mark.db
 def test_remove_and_add(database, module_type):
     """Tests adding and removing a tcl module file."""