Refactor members of PythonVersion

This makes the accessors more consistently named, makes the defaults/constants more obvious, and removes public access to a static mutable array.

Also do automated formatting fixes required by presubmit.

Work toward #6583.

RELNOTES: None
PiperOrigin-RevId: 223514946

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java

@@ -66,9 +66,11 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
           <a href="${link py_binary}"><code>py_binary</code></a> rules,
           <a href="${link py_library}"><code>py_library</code></a> rules.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-          .override(builder.copy("deps")
-              .legacyMandatoryProviders(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME)
-              .allowedFileTypes())
+          .override(
+              builder
+                  .copy("deps")
+                  .legacyMandatoryProviders(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME)
+                  .allowedFileTypes())
           /* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->
           List of import directories to be added to the <code>PYTHONPATH</code>.
           <p>
@@ -105,19 +107,21 @@ A string specifying the Python major version(s) that the <code>.py</code> source
             A synonym for PY3ONLY.<br/>
           <br/>
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-          .add(attr("srcs_version", STRING)
-              .value(PythonVersion.defaultSrcsVersion().toString())
-              .allowedValues(new AllowedValueSet(PythonVersion.getAllValues())))
+          .add(
+              attr("srcs_version", STRING)
+                  .value(PythonVersion.getDefaultSrcsValue().toString())
+                  .allowedValues(new AllowedValueSet(PythonVersion.getAllStrings())))
           // TODO(brandjon): Consider adding to py_interpreter a .mandatoryNativeProviders() of
           // BazelPyRuntimeProvider. (Add a test case to PythonConfigurationTest for violations
           // of this requirement.)
           .add(attr(":py_interpreter", LABEL).value(PY_INTERPRETER))
           // do not depend on lib2to3:2to3 rule, because it creates circular dependencies
           // 2to3 is itself written in Python and depends on many libraries.
-          .add(attr("$python2to3", LABEL)
-              .cfg(HostTransition.INSTANCE)
-              .exec()
-              .value(env.getToolsLabel("//tools/python:2to3")))
+          .add(
+              attr("$python2to3", LABEL)
+                  .cfg(HostTransition.INSTANCE)
+                  .exec()
+                  .value(env.getToolsLabel("//tools/python:2to3")))
           .setPreferredDependencyPredicate(PyRuleClasses.PYTHON_SOURCE)
           .build();
     }
@@ -163,8 +167,8 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(
               attr("default_python_version", STRING)
-                  .value(PythonVersion.defaultTargetPythonVersion().toString())
-                  .allowedValues(new AllowedValueSet(PythonVersion.getTargetPythonValues()))
+                  .value(PythonVersion.getDefaultTargetValue().toString())
+                  .allowedValues(new AllowedValueSet(PythonVersion.getTargetStrings()))
                   .nonconfigurable(
                       "read by PythonUtils.getNewPythonVersion, which doesn't have access"
                           + " to configuration keys"))

src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java

@@ -95,8 +95,8 @@ public PyCommon(RuleContext ruleContext) {
   }
 
   public void initCommon(PythonVersion defaultVersion) {
-    this.sourcesVersion = getPythonVersionAttr(
-        ruleContext, "srcs_version", PythonVersion.getAllVersions());
+    this.sourcesVersion =
+        getPythonVersionAttr(ruleContext, "srcs_version", PythonVersion.getAllValues());
 
     this.version = ruleContext.getFragment(PythonConfiguration.class)
         .getPythonVersion(defaultVersion);
@@ -217,7 +217,7 @@ public static PythonVersion getPythonVersionAttr(RuleContext ruleContext,
     ruleContext.attributeError(attrName,
         "'" + stringAttr + "' is not a valid value. Expected one of: " + Joiner.on(", ")
             .join(allowed));
-    return PythonVersion.defaultTargetPythonVersion();
+    return PythonVersion.getDefaultTargetValue();
   }
 
   /**

src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java

@@ -27,16 +27,19 @@ public class PyRuleClasses {
   public static final FileType PYTHON_SOURCE = FileType.of(".py", ".py3");
 
   /**
-   * Input for {@link RuleClass.Builder#cfg(RuleTransitionFactory)}: if
-   * {@link PythonOptions#forcePython} is unset, sets the Python version according to the rule's
-   * default Python version. Assumes the rule has the expected attribute for this setting.
+   * Input for {@link RuleClass.Builder#cfg(RuleTransitionFactory)}: if {@link
+   * PythonOptions#forcePython} is unset, sets the Python version according to the rule's default
+   * Python version. Assumes the rule has the expected attribute for this setting.
    *
    * <p>Since this is a configuration transition, this propagates to the rules' transitive deps.
    */
   public static final RuleTransitionFactory DEFAULT_PYTHON_VERSION_TRANSITION =
       (rule) ->
           new PythonVersionTransition(
+              // In case of a parse error, this will return null, which means that the transition
+              // would use the hard-coded default (PythonVersion#getDefaultTargetValue). But the
+              // attribute is already validated to allow only PythonVersion#getTargetStrings anyway.
               PythonVersion.parse(
                   RawAttributeMapper.of(rule).get("default_python_version", Type.STRING),
-                  PythonVersion.ALL_VALUES));
+                  PythonVersion.getAllValues()));
 }

src/main/java/com/google/devtools/build/lib/rules/python/PyTest.java

@@ -18,7 +18,6 @@
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.syntax.Type;
 
 /**
  * An implementation for {@code py_test} rules.
@@ -35,20 +34,13 @@ public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
     PythonSemantics semantics = createSemantics();
     PyCommon common = new PyCommon(ruleContext);
-    common.initCommon(getDefaultPythonVersion(ruleContext));
+    common.initCommon(common.getDefaultPythonVersion());
 
     RuleConfiguredTargetBuilder builder = PyBinary.init(ruleContext, semantics, common);
     if (builder == null) {
       return null;
     }
     return builder.build();
   }
-
-  private PythonVersion getDefaultPythonVersion(RuleContext ruleContext) {
-    return ruleContext.getRule().isAttrDefined("default_python_version", Type.STRING)
-        ? PyCommon.getPythonVersionAttr(ruleContext, "default_python_version", PythonVersion.PY2,
-            PythonVersion.PY3, PythonVersion.PY2AND3)
-        : null;
-  }
 }
 

src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java

@@ -65,7 +65,7 @@ public PythonVersion getPythonVersion(PythonVersion attributeVersion) {
 
   @Override
   public String getOutputDirectoryName() {
-    List<PythonVersion> allowedVersions = Arrays.asList(PythonVersion.TARGET_PYTHON_VALUES);
+    List<PythonVersion> allowedVersions = Arrays.asList(PythonVersion.getTargetValues());
     Verify.verify(
         allowedVersions.size() == 2, // If allowedVersions.size() == 1, we don't need this method.
         ">2 possible defaultPythonVersion values makes output directory clashes possible");

src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -63,7 +63,7 @@ public PythonVersionConverter() {
   public PythonVersion hostForcePython;
 
   public PythonVersion getPythonVersion() {
-    return getPythonVersion(PythonVersion.DEFAULT);
+    return getPythonVersion(PythonVersion.getDefaultTargetValue());
   }
 
   public PythonVersion getPythonVersion(PythonVersion defaultVersion) {

src/main/java/com/google/devtools/build/lib/rules/python/PythonVersion.java

@@ -11,6 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 package com.google.devtools.build.lib.rules.python;
 
 import static com.google.common.collect.Iterables.transform;
@@ -20,61 +21,133 @@
 import java.util.Arrays;
 
 /**
- * Python version for Python rules.
+ * An enum representing Python major versions.
+ *
+ * <p>This enum has two interpretations. The "target" interpretation is when this enum is used in a
+ * command line flag or in a rule attribute to denote a particular version of the Python language.
+ * Only {@code PY2} and {@code PY3} can be used as target values. The "sources" interpretation is
+ * when this enum is used to denote the degree of compatibility of source code with the target
+ * values.
  */
 public enum PythonVersion {
+
+  // TODO(#6445): Remove PY2ONLY and PY3ONLY.
+
+  /**
+   * Target value Python 2. Represents source code that is naturally compatible with Python 2.
+   *
+   * <p><i>Deprecated meaning:</i> Also indicates that source code is compatible with Python 3 under
+   * 2to3 transformation. 2to3 transformation is not implemented in Bazel and this meaning will be
+   * removed from Bazel (#1393).
+   */
   PY2,
+
+  /**
+   * Target value Python 3. Represents source code that is naturally compatible with Python 3.
+   *
+   * <p><i>Deprecated meaning:</i> Also indicates that source code is compatible with Python 2 under
+   * 3to2 transformation. 3to2 transformation was never implemented and this meaning should not be
+   * relied on.
+   */
   PY3,
+
+  /**
+   * Represents source code that is naturally compatible with both Python 2 and Python 3, i.e. code
+   * that lies in the intersection of both languages.
+   */
   PY2AND3,
-  PY2ONLY,
-  PY3ONLY;
 
-  static final PythonVersion[] ALL_VALUES =
-      new PythonVersion[] { PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY };
+  /**
+   * Alias for {@code PY2}. Deprecated in Bazel; prefer {@code PY2}.
+   *
+   * <p><i>Deprecated meaning:</i> Indicates code that cannot be processed by 2to3.
+   */
+  PY2ONLY,
 
   /**
-   * The Python version to use if not overridden by {@link PythonOptions#forcePython} or a rule's
-   * {@code default_python_version} or {@code srcs_version} attributes.
+   * Deprecated alias for {@code PY3}.
+   *
+   * <p><i>Deprecated meaning:</i> Indicates code that cannot be processed by 3to2.
    */
-  static final PythonVersion DEFAULT = PY2;
+  PY3ONLY;
 
-  static final PythonVersion[] NON_CONVERSION_VALUES =
-      new PythonVersion[] { PY2AND3, PY2ONLY, PY3ONLY };
+  private static Iterable<String> convertToStrings(PythonVersion[] values) {
+    return transform(ImmutableList.copyOf(values), Functions.toStringFunction());
+  }
+
+  private static final PythonVersion[] allValues =
+      new PythonVersion[] {PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY};
+
+  private static final Iterable<String> allStrings = convertToStrings(allValues);
+
+  private static final PythonVersion[] targetValues = new PythonVersion[] {PY2, PY3};
+
+  private static final Iterable<String> targetStrings = convertToStrings(targetValues);
 
-  static final PythonVersion[] TARGET_PYTHON_VALUES =
-      new PythonVersion[] { PY2, PY3 };
+  private static final PythonVersion[] nonConversionValues =
+      new PythonVersion[] {PY2AND3, PY2ONLY, PY3ONLY};
 
-  public static PythonVersion defaultSrcsVersion() {
-    return PY2AND3;
+  private static final Iterable<String> nonConversionStrings =
+      convertToStrings(nonConversionValues);
+
+  private static final PythonVersion DEFAULT_TARGET_VALUE = PY2;
+
+  private static final PythonVersion DEFAULT_SRCS_VALUE = PY2AND3;
+
+  /** Returns all values as a new array. */
+  public static PythonVersion[] getAllValues() {
+    return Arrays.copyOf(allValues, allValues.length);
   }
 
-  public static PythonVersion defaultTargetPythonVersion() {
-    return DEFAULT;
+  /** Returns an iterable of all values as strings. */
+  public static Iterable<String> getAllStrings() {
+    return allStrings;
   }
 
-  private static Iterable<String> convertToStrings(PythonVersion[] values) {
-    return transform(ImmutableList.copyOf(values), Functions.toStringFunction());
+  /** Returns all values representing a specific version, as a new array. */
+  public static PythonVersion[] getTargetValues() {
+    return Arrays.copyOf(targetValues, targetValues.length);
   }
 
-  public static PythonVersion[] getAllVersions() {
-    return ALL_VALUES;
+  /** Returns an iterable of all values representing a specific version, as strings. */
+  public static Iterable<String> getTargetStrings() {
+    return targetStrings;
   }
 
-  public static Iterable<String> getAllValues() {
-    return convertToStrings(ALL_VALUES);
+  /**
+   * Returns all values that do not imply running a transpiler to convert between versions, as a new
+   * array.
+   */
+  public static PythonVersion[] getNonConversionValues() {
+    return Arrays.copyOf(nonConversionValues, nonConversionValues.length);
   }
 
-  public static Iterable<String> getNonConversionValues() {
-    return convertToStrings(NON_CONVERSION_VALUES);
+  /**
+   * Returns all values that do not imply running a transpiler to convert between versions, as
+   * strings.
+   */
+  public static Iterable<String> getNonConversionStrings() {
+    return nonConversionStrings;
   }
 
-  public static Iterable<String> getTargetPythonValues() {
-    return convertToStrings(TARGET_PYTHON_VALUES);
+  /** Returns the Python version to use if not otherwise specified by a flag or attribute. */
+  public static PythonVersion getDefaultTargetValue() {
+    return DEFAULT_TARGET_VALUE;
+  }
+
+  /**
+   * Returns the level of source compatibility assumed if not otherwise specified by an attribute.
+   */
+  public static PythonVersion getDefaultSrcsValue() {
+    return DEFAULT_SRCS_VALUE;
   }
 
+  // TODO(brandjon): Refactor this into parseTargetValue and parseSourcesValue methods. Throw
+  // IllegalArgumentException on bad values instead of returning null, and modify callers to
+  // tolerate the exception.
   /**
-   * Converts the string to PythonVersion, if it is one of the allowed values.
-   * Returns null if the input is not valid.
+   * Converts the string to PythonVersion, if it is one of the allowed values. Returns null if the
+   * input is not valid.
    */
   public static PythonVersion parse(String str, PythonVersion... allowed) {
     if (str == null) {

src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java

@@ -172,7 +172,7 @@ public void srcsVersionClashesWithDefaultVersionAttr() throws Exception {
   public void srcsVersionClashesWithDefaultVersionAttr_Implicitly() throws Exception {
     // Canary assertion: This'll fail when we flip the default to PY3. At that point change this
     // test to use srcs_version = 'PY2ONLY' instead.
-    assertThat(PythonVersion.defaultTargetPythonVersion()).isEqualTo(PythonVersion.PY2);
+    assertThat(PythonVersion.getDefaultTargetValue()).isEqualTo(PythonVersion.PY2);
 
     // Fails because default_python_version is PY2 by default, so the config is set to PY2
     // regardless of srcs_version.