Use both the deprecated and new locations of fatal_warnings args (#6798)

### Problem

Currently it is necessary to empty the `fatal_warnings_(enabled|disabled)_args` list in order for the values specified in `compiler_options_sets` to be used. But emptying the list (rather than using its default) requires actually specifying the deprecated options, which triggers a deprecation warning.

### Solution

Include either the values from `fatal_warnings_(enabled|disabled)_args` and `compiler_options_sets` depending on which has been explicitly set, to ensure that the options are consumed in either situation.

### Result

It's possible to migrate to `compiler_options_sets` without triggering a deprecation warning.

src/python/pants/option/compiler_option_sets_mixin.py

@@ -6,6 +6,7 @@
 
 from builtins import object
 
+from pants.util.memo import memoized_property
 from pants.util.meta import classproperty
 
 
@@ -53,29 +54,42 @@ def get_fatal_warnings_disabled_args_default(cls):
     """Override to set default for this option."""
     return ()
 
+  @memoized_property
+  def _use_deprecated_fatal_warnings(self):
+    """Returns true if fatal warnings should be used from their deprecated location.
+
+    The deprecated location is used only if it is explicitly specified, and no args were explicitly
+    specified in the new location. This means that either one location or the other will be used,
+    but never both.
+    """
+    set_in_deprecated_location = not self.get_options().is_default('fatal_warnings_enabled_args') or \
+                                 not self.get_options().is_default('fatal_warnings_disabled_args')
+    set_enabled_in_new_location = (not self.get_options().is_default('compiler_option_sets_enabled_args') and \
+                                   bool(self.get_options().compiler_option_sets_enabled_args.get('fatal_warnings', None)))
+    set_disabled_in_new_location = (not self.get_options().is_default('compiler_option_sets_disabled_args') and \
+                                   bool(self.get_options().compiler_option_sets_disabled_args.get('fatal_warnings', None)))
+    return set_in_deprecated_location and not (set_enabled_in_new_location or set_disabled_in_new_location)
+
   def get_merged_args_for_compiler_option_sets(self, compiler_option_sets):
     compiler_options = set()
 
-    # Set values for enabled options.
-    for option_set_key in compiler_option_sets:
-      # Fatal warnings option has special treatment for backwards compatibility.
-      # This is because previously this has had its own {enabled, disabled}_args
-      # options when these were defined in the jvm compile task.
-      fatal_warnings = self.get_options().fatal_warnings_enabled_args
-      if option_set_key == 'fatal_warnings' and fatal_warnings:
-        compiler_options.update(fatal_warnings)
+    # Start by setting the (deprecated) magically handled fatal warnings option.
+    if self._use_deprecated_fatal_warnings:
+      if 'fatal_warnings' in compiler_option_sets:
+        compiler_options.update(self.get_options().fatal_warnings_enabled_args)
       else:
+        compiler_options.update(self.get_options().fatal_warnings_disabled_args)
+
+    # Set values for enabled options (ignoring fatal_warnings if it has been handled above).
+    for option_set_key in compiler_option_sets:
+      if option_set_key != 'fatal_warnings' or not self._use_deprecated_fatal_warnings:
         val = self.get_options().compiler_option_sets_enabled_args.get(option_set_key, ())
         compiler_options.update(val)
 
-    # Set values for disabled options.
-    for option_set, disabled_args in self.get_options().compiler_option_sets_disabled_args.items():
-      # Fatal warnings option has special treatment for backwards compatibility.
-      disabled_fatal_warn_args = self.get_options().fatal_warnings_disabled_args
-      if option_set == 'fatal_warnings' and disabled_fatal_warn_args:
-        compiler_options.update(disabled_fatal_warn_args)
-      else:
-        if not option_set in compiler_option_sets:
+    # Set values for disabled options (ignoring fatal_warnings if it has been handled above).
+    for option_set_key, disabled_args in self.get_options().compiler_option_sets_disabled_args.items():
+      if not option_set_key in compiler_option_sets:
+        if option_set_key != 'fatal_warnings' or not self._use_deprecated_fatal_warnings:
           compiler_options.update(disabled_args)
 
     return list(compiler_options)

tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py

@@ -124,7 +124,7 @@ def test_zinc_unsupported_option(self):
         # Confirm that we were warned.
         self.assertIn('is not supported, and is subject to change/removal', pants_run.stdout_data)
 
-  def test_zinc_fatal_warning(self):
+  def test_zinc_fatal_warnings(self):
     def test_combination(target, expect_success, extra_args=[]):
       with self.temporary_workdir() as workdir:
         with self.temporary_cachedir() as cachedir:
@@ -147,6 +147,29 @@ def test_combination(target, expect_success, extra_args=[]):
     test_combination('fatal', expect_success=False,
       extra_args=['--compile-zinc-fatal-warnings-disabled-args=[\'-S-Xfatal-warnings\']'])
 
+  def test_zinc_compiler_options_sets(self):
+    def test_combination(target, expect_success, extra_args=[]):
+      with self.temporary_workdir() as workdir:
+        with self.temporary_cachedir() as cachedir:
+          pants_run = self.run_test_compile(
+              workdir,
+              cachedir,
+              'testprojects/src/scala/org/pantsbuild/testproject/compilation_warnings:{}'.format(
+                target),
+              extra_args=extra_args)
+
+          if expect_success:
+            self.assert_success(pants_run)
+          else:
+            self.assert_failure(pants_run)
+    test_combination('fatal', expect_success=False)
+    test_combination('nonfatal', expect_success=True)
+
+    test_combination('fatal', expect_success=True,
+      extra_args=['--compile-zinc-compiler-option-sets-enabled-args={"fatal_warnings": ["-C-Werror"]}'])
+    test_combination('fatal', expect_success=False,
+      extra_args=['--compile-zinc-compiler-option-sets-disabled-args={"fatal_warnings": ["-S-Xfatal-warnings"]}'])
+
   @unittest.expectedFailure
   def test_soft_excludes_at_compiletime(self):
     with self.do_test_compile('testprojects/src/scala/org/pantsbuild/testproject/exclude_direct_dep',