Skip to content

Commit

Permalink
Cleanup PythonVersion by changing accessors to immutable public fields
Browse files Browse the repository at this point in the history
No need to have these mutable arrays lying around and all this extra boilerplate. It's easier to read if we just use static constant fields.

Work toward #6583.

RELNOTES: None
PiperOrigin-RevId: 223806539
  • Loading branch information
brandjon authored and Copybara-Service committed Dec 3, 2018
1 parent 9d81cf7 commit 3123cfe
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ A string specifying the Python major version(s) that the <code>.py</code> source
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("srcs_version", STRING)
.value(PythonVersion.getDefaultSrcsValue().toString())
.allowedValues(new AllowedValueSet(PythonVersion.getAllStrings())))
.value(PythonVersion.DEFAULT_SRCS_VALUE.toString())
.allowedValues(new AllowedValueSet(PythonVersion.ALL_STRINGS)))
// TODO(brandjon): Consider adding to py_interpreter a .mandatoryNativeProviders() of
// BazelPyRuntimeProvider. (Add a test case to PythonConfigurationTest for violations
// of this requirement.)
Expand Down Expand Up @@ -167,8 +167,8 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("default_python_version", STRING)
.value(PythonVersion.getDefaultTargetValue().toString())
.allowedValues(new AllowedValueSet(PythonVersion.getTargetStrings()))
.value(PythonVersion.DEFAULT_TARGET_VALUE.toString())
.allowedValues(new AllowedValueSet(PythonVersion.TARGET_STRINGS))
.nonconfigurable(
"read by PythonUtils.getNewPythonVersion, which doesn't have access"
+ " to configuration keys"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ private static PythonVersion getSrcsVersionAttr(RuleContext ruleContext) {
"srcs_version",
String.format(
"'%s' is not a valid value. Expected one of: %s",
attrValue, Joiner.on(", ").join(PythonVersion.getAllStrings())));
return PythonVersion.getDefaultSrcsValue();
attrValue, Joiner.on(", ").join(PythonVersion.ALL_STRINGS)));
return PythonVersion.DEFAULT_SRCS_VALUE;
}
}

Expand All @@ -227,8 +227,8 @@ private static PythonVersion getPythonVersionAttr(RuleContext ruleContext) {
"default_python_version",
String.format(
"'%s' is not a valid value. Expected one of: %s",
attrValue, Joiner.on(", ").join(PythonVersion.getTargetStrings())));
return PythonVersion.getDefaultTargetValue();
attrValue, Joiner.on(", ").join(PythonVersion.TARGET_STRINGS)));
return PythonVersion.DEFAULT_TARGET_VALUE;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.common.options.TriState;
import java.util.Arrays;
import java.util.List;

/**
Expand Down Expand Up @@ -65,7 +64,7 @@ public PythonVersion getPythonVersion(PythonVersion attributeVersion) {

@Override
public String getOutputDirectoryName() {
List<PythonVersion> allowedVersions = Arrays.asList(PythonVersion.getTargetValues());
List<PythonVersion> allowedVersions = PythonVersion.TARGET_VALUES;
Verify.verify(
allowedVersions.size() == 2, // If allowedVersions.size() == 1, we don't need this method.
">2 possible defaultPythonVersion values makes output directory clashes possible");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public PythonVersionConverter() {
public PythonVersion hostForcePython;

public PythonVersion getPythonVersion() {
return getPythonVersion(PythonVersion.getDefaultTargetValue());
return getPythonVersion(PythonVersion.DEFAULT_TARGET_VALUE);
}

public PythonVersion getPythonVersion(PythonVersion defaultVersion) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import com.google.common.base.Functions;
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
import java.util.List;

/**
* An enum representing Python major versions.
Expand Down Expand Up @@ -69,78 +69,38 @@ public enum PythonVersion {
*/
PY3ONLY;

private static ImmutableList<String> convertToStrings(PythonVersion[] values) {
return Arrays.stream(values)
private static ImmutableList<String> convertToStrings(List<PythonVersion> values) {
return values.stream()
.map(Functions.toStringFunction())
.collect(ImmutableList.toImmutableList());
}

private static final PythonVersion[] allValues =
new PythonVersion[] {PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY};
/** All enum values. */
public static final ImmutableList<PythonVersion> ALL_VALUES =
ImmutableList.of(PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY);

private static final ImmutableList<String> ALL_STRINGS = convertToStrings(allValues);
/** String names of all enum values. */
public static final ImmutableList<String> ALL_STRINGS = convertToStrings(ALL_VALUES);

private static final PythonVersion[] targetValues = new PythonVersion[] {PY2, PY3};
/** Enum values representing a distinct Python version. */
public static final ImmutableList<PythonVersion> TARGET_VALUES = ImmutableList.of(PY2, PY3);

private static final ImmutableList<String> TARGET_STRINGS = convertToStrings(targetValues);
/** String names of enum values representing a distinct Python version. */
public static final ImmutableList<String> TARGET_STRINGS = convertToStrings(TARGET_VALUES);

private static final PythonVersion[] nonConversionValues =
new PythonVersion[] {PY2AND3, PY2ONLY, PY3ONLY};

private static final ImmutableList<String> NON_CONVERSION_STRINGS =
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);
}

/** Returns an iterable of all values as strings. */
public static ImmutableList<String> getAllStrings() {
return ALL_STRINGS;
}

/** Returns all values representing a specific version, as a new array. */
public static PythonVersion[] getTargetValues() {
return Arrays.copyOf(targetValues, targetValues.length);
}

/** Returns an iterable of all values representing a specific version, as strings. */
public static ImmutableList<String> getTargetStrings() {
return TARGET_STRINGS;
}

/**
* 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);
}
/** Enum values that do not imply running a transpiler to convert between versions. */
public static final ImmutableList<PythonVersion> NON_CONVERSION_VALUES =
ImmutableList.of(PY2AND3, PY2ONLY, PY3ONLY);

/**
* Returns all values that do not imply running a transpiler to convert between versions, as
* strings.
* String names of enum values that do not imply running a transpiler to convert between versions.
*/
public static ImmutableList<String> getNonConversionStrings() {
return NON_CONVERSION_STRINGS;
}
public static final ImmutableList<String> NON_CONVERSION_STRINGS =
convertToStrings(NON_CONVERSION_VALUES);

/** Returns the Python version to use if not otherwise specified by a flag or attribute. */
public static PythonVersion getDefaultTargetValue() {
return DEFAULT_TARGET_VALUE;
}
public static final PythonVersion DEFAULT_TARGET_VALUE = PY2;

/**
* Returns the level of source compatibility assumed if not otherwise specified by an attribute.
*/
public static PythonVersion getDefaultSrcsValue() {
return DEFAULT_SRCS_VALUE;
}
public static final PythonVersion DEFAULT_SRCS_VALUE = PY2AND3;

/**
* Converts the string to a target {@code PythonVersion} value (case-sensitive).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.getDefaultTargetValue()).isEqualTo(PythonVersion.PY2);
assertThat(PythonVersion.DEFAULT_TARGET_VALUE).isEqualTo(PythonVersion.PY2);

// Fails because default_python_version is PY2 by default, so the config is set to PY2
// regardless of srcs_version.
Expand Down

0 comments on commit 3123cfe

Please sign in to comment.