Bug 1492716 - Part 3: Add linter for configure option with bool default. r=glandium
authorTooru Fujisawa <arai_a@mac.com>
Tue, 16 Oct 2018 20:28:12 +0900
changeset 445349 6630144571e3e9e28ae51d6dbede2d70fafc5e9e
parent 445348 561373eda70f3fafcbea47304fcdd106aba12a83
child 445350 9ea543fc770afe79edc4c11be6d83bb721c0ee18
push id109719
push userarai_a@mac.com
push dateFri, 09 Nov 2018 06:25:57 +0000
treeherdermozilla-inbound@a922413f0f74 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1492716
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1492716 - Part 3: Add linter for configure option with bool default. r=glandium Differential Revision: https://phabricator.services.mozilla.com/D8835
python/mozbuild/mozbuild/configure/lint.py
python/mozbuild/mozbuild/test/configure/test_lint.py
--- a/python/mozbuild/mozbuild/configure/lint.py
+++ b/python/mozbuild/mozbuild/configure/lint.py
@@ -1,37 +1,42 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import, print_function, unicode_literals
 
 import inspect
+import types
 from functools import wraps
 from StringIO import StringIO
 from . import (
     CombinedDependsFunction,
     ConfigureError,
     ConfigureSandbox,
     DependsFunction,
     SandboxedGlobal,
     TrivialDependsFunction,
+    SandboxDependsFunction,
 )
 from .lint_util import disassemble_as_iter
 from mozbuild.util import memoize
 
 
 class LintSandbox(ConfigureSandbox):
     def __init__(self, environ=None, argv=None, stdout=None, stderr=None):
         out = StringIO()
         stdout = stdout or out
         stderr = stderr or out
         environ = environ or {}
         argv = argv or []
         self._wrapped = {}
+        self._bool_options = []
+        self._bool_func_options = []
+        self.LOG = ""
         super(LintSandbox, self).__init__({}, environ=environ, argv=argv,
                                           stdout=stdout, stderr=stderr)
 
     def run(self, path=None):
         if path:
             self.include_file(path)
 
         for dep in self._depends.itervalues():
@@ -117,18 +122,51 @@ class LintSandbox(ConfigureSandbox):
         return super(LintSandbox, self)._value_for_depends(
             obj, need_help_dependency)
 
     def option_impl(self, *args, **kwargs):
         result = super(LintSandbox, self).option_impl(*args, **kwargs)
         when = self._conditions.get(result)
         if when:
             self._value_for(when, need_help_dependency=True)
+
+        self._check_option(*args, **kwargs)
+
         return result
 
+    def _check_option(self, *args, **kwargs):
+        if 'default' not in kwargs:
+            return
+        if len(args) == 0:
+            return
+
+        name = args[0]
+        default = kwargs['default']
+
+        if type(default) != bool:
+            return
+
+        table = {
+            True: {
+                'enable': 'disable',
+                'with': 'without',
+            },
+            False: {
+                'disable': 'enable',
+                'without': 'with',
+            }
+        }
+        for prefix, replacement in table[default].iteritems():
+            if name.startswith('--{}-'.format(prefix)):
+                raise ConfigureError(('{} should be used instead of '
+                                      '{} with default={}').format(
+                                          name.replace('--{}-'.format(prefix),
+                                                       '--{}-'.format(replacement)),
+                                          name, default))
+
     def unwrap(self, func):
         glob = func.func_globals
         while func in self._wrapped:
             if isinstance(func.func_globals, SandboxedGlobal):
                 glob = func.func_globals
             func = self._wrapped[func]
         return func, glob
 
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -256,11 +256,71 @@ class TestLint(unittest.TestCase):
                 include(foo)
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "%s:9: The dependency on `qux` is unused."
                           % mozpath.join(test_data_path, 'moz.configure'))
 
+    def test_default_enable(self):
+        # --enable-* with default=True is not allowed.
+        with self.moz_configure('''
+            option('--enable-foo', default=False, help='foo')
+        '''):
+            self.lint_test()
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--enable-foo', default=True, help='foo')
+            '''):
+                self.lint_test()
+        self.assertEquals(e.exception.message,
+                          '--disable-foo should be used instead of '
+                          '--enable-foo with default=True')
+
+    def test_default_disable(self):
+        # --disable-* with default=False is not allowed.
+        with self.moz_configure('''
+            option('--disable-foo', default=True, help='foo')
+        '''):
+            self.lint_test()
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--disable-foo', default=False, help='foo')
+            '''):
+                self.lint_test()
+        self.assertEquals(e.exception.message,
+                          '--enable-foo should be used instead of '
+                          '--disable-foo with default=False')
+
+    def test_default_with(self):
+        # --with-* with default=True is not allowed.
+        with self.moz_configure('''
+            option('--with-foo', default=False, help='foo')
+        '''):
+            self.lint_test()
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--with-foo', default=True, help='foo')
+            '''):
+                self.lint_test()
+        self.assertEquals(e.exception.message,
+                          '--without-foo should be used instead of '
+                          '--with-foo with default=True')
+
+    def test_default_without(self):
+        # --without-* with default=False is not allowed.
+        with self.moz_configure('''
+            option('--without-foo', default=True, help='foo')
+        '''):
+            self.lint_test()
+        with self.assertRaises(ConfigureError) as e:
+            with self.moz_configure('''
+                option('--without-foo', default=False, help='foo')
+            '''):
+                self.lint_test()
+        self.assertEquals(e.exception.message,
+                          '--with-foo should be used instead of '
+                          '--without-foo with default=False')
+
 
 if __name__ == '__main__':
     main()