Skip to content

Commit

Permalink
Remove the publicByDefault attribute of rules
Browse files Browse the repository at this point in the history
It is no longer used (and probably shouldn't be used).

PiperOrigin-RevId: 326417219
  • Loading branch information
laurentlb authored and copybara-github committed Aug 13, 2020
1 parent 814c3da commit 40e7636
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.docgen.DocgenConsts.RuleType;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.packages.RuleClass;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -54,7 +52,6 @@ public class RuleDocumentation implements Comparable<RuleDocumentation> {
private final Map<String, String> docVariables = new HashMap<>();
// Only one attribute per attributeName is allowed
private final Set<RuleDocumentationAttribute> attributes = new TreeSet<>();
private final ConfiguredRuleClassProvider ruleClassProvider;

private RuleLinkExpander linkExpander;

Expand All @@ -75,7 +72,6 @@ public class RuleDocumentation implements Comparable<RuleDocumentation> {
int startLineCount,
String fileName,
ImmutableSet<String> flags,
ConfiguredRuleClassProvider ruleClassProvider,
String familySummary)
throws BuildEncyclopediaDocException {
Preconditions.checkNotNull(ruleName);
Expand All @@ -91,7 +87,6 @@ public class RuleDocumentation implements Comparable<RuleDocumentation> {
this.startLineCount = startLineCount;
this.fileName = fileName;
this.flags = flags;
this.ruleClassProvider = ruleClassProvider;
this.familySummary = familySummary;
}

Expand All @@ -102,8 +97,7 @@ public class RuleDocumentation implements Comparable<RuleDocumentation> {
String htmlDocumentation,
int startLineCount,
String fileName,
ImmutableSet<String> flags,
ConfiguredRuleClassProvider ruleClassProvider)
ImmutableSet<String> flags)
throws BuildEncyclopediaDocException {
this(
ruleName,
Expand All @@ -113,7 +107,6 @@ public class RuleDocumentation implements Comparable<RuleDocumentation> {
startLineCount,
fileName,
flags,
ruleClassProvider,
"");
}

Expand Down Expand Up @@ -266,14 +259,6 @@ public String getNameExtraHtmlDoc() throws BuildEncyclopediaDocException {
return expandedDoc;
}

/**
* Returns whether this rule has public visibility by default.
*/
public boolean isPublicByDefault() {
RuleClass ruleClass = ruleClassProvider.getRuleClassMap().get(ruleName);
return ruleClass != null && ruleClass.isPublicByDefault();
}

/**
* Returns whether this rule is deprecated.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ private void endBlazeRuleDoc(final Map<String, RuleDocumentation> documentations
getLineCnt(),
javaSourceFilePath,
flags,
ruleClassProvider,
familySummary));
sb = new StringBuilder();
inBlazeRuleDocs = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ ${ruleFamily.summary}
#end
</tbody>
</table>
#if ($rule.isPublicByDefault())
The default visibility is public: <code>visibility = ["//visibility:public"]</code>.
#end
#end

#if (!$singlePage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,10 +664,6 @@ public RuleVisibility getVisibility() {
return visibility;
}

if (getRuleClassObject().isPublicByDefault()) {
return ConstantRuleVisibility.PUBLIC;
}

return pkg.getDefaultVisibility();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,6 @@ public String toString() {
private final boolean starlark;
private boolean starlarkTestable = false;
private boolean documented;
private boolean publicByDefault = false;
private boolean binaryOutput = true;
private boolean workspaceOnly = false;
private boolean isExecutableStarlark = false;
Expand Down Expand Up @@ -875,7 +874,6 @@ public RuleClass build(String name, String key) {
starlark,
starlarkTestable,
documented,
publicByDefault,
binaryOutput,
workspaceOnly,
isExecutableStarlark,
Expand Down Expand Up @@ -1048,11 +1046,6 @@ public Builder setUndocumented() {
return this;
}

public Builder publicByDefault() {
publicByDefault = true;
return this;
}

public Builder setWorkspaceOnly() {
workspaceOnly = true;
return this;
Expand Down Expand Up @@ -1557,7 +1550,6 @@ public Attribute.Builder<?> copy(String name) {
private final boolean isStarlark;
private final boolean starlarkTestable;
private final boolean documented;
private final boolean publicByDefault;
private final boolean binaryOutput;
private final boolean workspaceOnly;
private final boolean isExecutableStarlark;
Expand Down Expand Up @@ -1695,7 +1687,6 @@ public Attribute.Builder<?> copy(String name) {
boolean isStarlark,
boolean starlarkTestable,
boolean documented,
boolean publicByDefault,
boolean binaryOutput,
boolean workspaceOnly,
boolean isExecutableStarlark,
Expand Down Expand Up @@ -1734,7 +1725,6 @@ public Attribute.Builder<?> copy(String name) {
this.targetKind = name + Rule.targetKindSuffix();
this.starlarkTestable = starlarkTestable;
this.documented = documented;
this.publicByDefault = publicByDefault;
this.binaryOutput = binaryOutput;
this.implicitOutputsFunction = implicitOutputsFunction;
this.transitionFactory = transitionFactory;
Expand Down Expand Up @@ -2550,10 +2540,6 @@ public boolean isDocumented() {
return documented;
}

public boolean isPublicByDefault() {
return publicByDefault;
}

/**
* Returns true iff the outputs of this rule should be created beneath the
* <i>bin</i> directory, false if beneath <i>genfiles</i>. For most rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public static Build.Rule.Builder serializeRule(Rule rule) {
Build.Rule.Builder builder = Build.Rule.newBuilder();
builder.setName(rule.getLabel().getName());
builder.setRuleClass(rule.getRuleClass());
builder.setPublicByDefault(rule.getRuleClassObject().isPublicByDefault());

RawAttributeMapper rawAttributeMapper = RawAttributeMapper.of(rule);
boolean isStarlark = rule.getRuleClassObject().isStarlark();
Expand Down
2 changes: 1 addition & 1 deletion src/main/protobuf/build.proto
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ message Rule {
repeated string default_setting = 7;

// The rule's class's public by default value.
optional bool public_by_default = 9;
optional bool DEPRECATED_public_by_default = 9;

optional bool DEPRECATED_is_skylark = 10;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.docgen.testutil.TestData.TestRule;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -32,8 +30,6 @@
public class RuleDocumentationTest {

private static final ImmutableSet<String> NO_FLAGS = ImmutableSet.<String>of();
private static final ConfiguredRuleClassProvider provider =
TestRuleClassProvider.getRuleClassProvider();

private static void assertContains(String base, String value) {
assertWithMessage(base + " is expected to contain " + value)
Expand All @@ -58,12 +54,15 @@ private void checkAttributeForRule(RuleDocumentation rule, RuleDocumentationAttr

@Test
public void testVariableSubstitution() throws BuildEncyclopediaDocException {
RuleDocumentation ruleDoc = new RuleDocumentation(
"rule", "OTHER", "FOO", Joiner.on("\n").join(new String[] {
"x",
"${VAR}",
"z"}),
0, "", ImmutableSet.<String>of(), provider);
RuleDocumentation ruleDoc =
new RuleDocumentation(
"rule",
"OTHER",
"FOO",
Joiner.on("\n").join(new String[] {"x", "${VAR}", "z"}),
0,
"",
ImmutableSet.<String>of());
ruleDoc.addDocVariable("VAR", "y");
assertThat(ruleDoc.getHtmlDocumentation()).isEqualTo("x\ny\nz");
}
Expand All @@ -74,8 +73,9 @@ public void testSignatureContainsCommonAttribute() throws Exception {
"licenses", "common", "attribute doc");
checkAttributeForRule(
new RuleDocumentation(
"java_binary", "BINARY", "JAVA", "", 0, "", ImmutableSet.<String>of(), provider),
licensesAttr, true);
"java_binary", "BINARY", "JAVA", "", 0, "", ImmutableSet.<String>of()),
licensesAttr,
true);
}

@Test
Expand All @@ -84,31 +84,36 @@ public void testInheritedAttributeGeneratesSignature() throws Exception {
"runtime_deps", "attribute doc", 0, "", NO_FLAGS);
checkAttributeForRule(
new RuleDocumentation(
"java_binary", "BINARY", "JAVA", "", 0, "", ImmutableSet.<String>of(), provider),
runtimeDepsAttr, false);
"java_binary", "BINARY", "JAVA", "", 0, "", ImmutableSet.<String>of()),
runtimeDepsAttr,
false);
checkAttributeForRule(
new RuleDocumentation(
"java_library", "LIBRARY", "JAVA", "", 0, "", ImmutableSet.<String>of(), provider),
runtimeDepsAttr, false);
"java_library", "LIBRARY", "JAVA", "", 0, "", ImmutableSet.<String>of()),
runtimeDepsAttr,
false);
}

@Test
public void testRuleDocFlagSubstitution() throws BuildEncyclopediaDocException {
RuleDocumentation ruleDoc = new RuleDocumentation(
"rule", "OTHER", "FOO", "x", 0, "", ImmutableSet.<String>of("DEPRECATED"), provider);
RuleDocumentation ruleDoc =
new RuleDocumentation(
"rule", "OTHER", "FOO", "x", 0, "", ImmutableSet.<String>of("DEPRECATED"));
ruleDoc.addDocVariable("VAR", "y");
assertThat(ruleDoc.getHtmlDocumentation()).isEqualTo("x");
}

@Test
public void testCommandLineDocumentation() throws BuildEncyclopediaDocException {
RuleDocumentation ruleDoc = new RuleDocumentation(
"foo_binary", "OTHER", "FOO", Joiner.on("\n").join(new String[] {
"x",
"y",
"z",
"${VAR}"}),
0, "", ImmutableSet.<String>of(), provider);
RuleDocumentation ruleDoc =
new RuleDocumentation(
"foo_binary",
"OTHER",
"FOO",
Joiner.on("\n").join(new String[] {"x", "y", "z", "${VAR}"}),
0,
"",
ImmutableSet.<String>of());
ruleDoc.addDocVariable("VAR", "w");
RuleDocumentationAttribute attributeDoc = RuleDocumentationAttribute.create(TestRule.class,
"srcs", "attribute doc", 0, "", NO_FLAGS);
Expand All @@ -118,60 +123,63 @@ public void testCommandLineDocumentation() throws BuildEncyclopediaDocException

@Test
public void testExtractExamples() throws BuildEncyclopediaDocException {
RuleDocumentation ruleDoc = new RuleDocumentation(
"rule", "OTHER", "FOO", Joiner.on("\n").join(new String[] {
"x",
"<!-- #BLAZE_RULE.EXAMPLE -->",
"a",
"<!-- #BLAZE_RULE.END_EXAMPLE -->",
"y",
"<!-- #BLAZE_RULE.EXAMPLE -->",
"b",
"<!-- #BLAZE_RULE.END_EXAMPLE -->",
"z"}),
0, "", ImmutableSet.<String>of(), provider);
RuleDocumentation ruleDoc =
new RuleDocumentation(
"rule",
"OTHER",
"FOO",
Joiner.on("\n")
.join(
new String[] {
"x",
"<!-- #BLAZE_RULE.EXAMPLE -->",
"a",
"<!-- #BLAZE_RULE.END_EXAMPLE -->",
"y",
"<!-- #BLAZE_RULE.EXAMPLE -->",
"b",
"<!-- #BLAZE_RULE.END_EXAMPLE -->",
"z"
}),
0,
"",
ImmutableSet.<String>of());
assertThat(ruleDoc.extractExamples()).isEqualTo(ImmutableSet.<String>of("a\n", "b\n"));
}

@Test
public void testCreateExceptions() throws BuildEncyclopediaDocException {
RuleDocumentation ruleDoc = new RuleDocumentation(
"foo_binary", "OTHER", "FOO", "", 10, "foo.txt", NO_FLAGS, provider);
RuleDocumentation ruleDoc =
new RuleDocumentation("foo_binary", "OTHER", "FOO", "", 10, "foo.txt", NO_FLAGS);
BuildEncyclopediaDocException e = ruleDoc.createException("msg");
assertThat(e).hasMessageThat().isEqualTo("Error in foo.txt:10: msg");
}

@Test
public void testEquals() throws BuildEncyclopediaDocException {
assertThat(new RuleDocumentation("rule", "OTHER", "FOO", "y", 0, "", NO_FLAGS, provider))
.isEqualTo(new RuleDocumentation("rule", "OTHER", "FOO", "x", 0, "", NO_FLAGS, provider));
assertThat(new RuleDocumentation("rule", "OTHER", "FOO", "y", 0, "", NO_FLAGS))
.isEqualTo(new RuleDocumentation("rule", "OTHER", "FOO", "x", 0, "", NO_FLAGS));
}

@Test
public void testNotEquals() throws BuildEncyclopediaDocException {
assertThat(
new RuleDocumentation("rule1", "OTHER", "FOO", "x", 0, "", NO_FLAGS, provider)
.equals(
new RuleDocumentation("rule2", "OTHER", "FOO", "y", 0, "", NO_FLAGS, provider)))
new RuleDocumentation("rule1", "OTHER", "FOO", "x", 0, "", NO_FLAGS)
.equals(new RuleDocumentation("rule2", "OTHER", "FOO", "y", 0, "", NO_FLAGS)))
.isFalse();
}

@Test
public void testCompareTo() throws BuildEncyclopediaDocException {
assertThat(
new RuleDocumentation("rule1", "OTHER", "FOO", "x", 0, "", NO_FLAGS, provider)
.compareTo(
new RuleDocumentation("rule2", "OTHER", "FOO", "x", 0, "", NO_FLAGS, provider)))
new RuleDocumentation("rule1", "OTHER", "FOO", "x", 0, "", NO_FLAGS)
.compareTo(new RuleDocumentation("rule2", "OTHER", "FOO", "x", 0, "", NO_FLAGS)))
.isEqualTo(-1);
}

@Test
public void testHashCode() throws BuildEncyclopediaDocException {
assertThat(
new RuleDocumentation("rule", "OTHER", "FOO", "y", 0, "", NO_FLAGS, provider)
.hashCode())
.isEqualTo(
new RuleDocumentation("rule", "OTHER", "FOO", "x", 0, "", NO_FLAGS, provider)
.hashCode());
assertThat(new RuleDocumentation("rule", "OTHER", "FOO", "y", 0, "", NO_FLAGS).hashCode())
.isEqualTo(new RuleDocumentation("rule", "OTHER", "FOO", "x", 0, "", NO_FLAGS).hashCode());
}
}
Loading

0 comments on commit 40e7636

Please sign in to comment.