Simplify visibility rules that include //visibility:public am: 44885e29d6

Change-Id: Ia189b5486bc51b3b6a399f1cde66a2030c70c557

android/visibility.go

@@ -186,8 +186,8 @@ func (r privateRule) String() string {
 var visibilityRuleMap = NewOnceKey("visibilityRuleMap")
 
 // The map from qualifiedModuleName to visibilityRule.
-func moduleToVisibilityRuleMap(ctx BaseModuleContext) *sync.Map {
-	return ctx.Config().Once(visibilityRuleMap, func() interface{} {
+func moduleToVisibilityRuleMap(config Config) *sync.Map {
+	return config.Once(visibilityRuleMap, func() interface{} {
 		return &sync.Map{}
 	}).(*sync.Map)
 }
@@ -304,7 +304,7 @@ func visibilityRuleGatherer(ctx BottomUpMutatorContext) {
 	if visibility := m.visibility(); visibility != nil {
 		rule := parseRules(ctx, currentPkg, m.visibility())
 		if rule != nil {
-			moduleToVisibilityRuleMap(ctx).Store(qualifiedModuleId, rule)
+			moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, rule)
 		}
 	}
 }
@@ -312,6 +312,7 @@ func visibilityRuleGatherer(ctx BottomUpMutatorContext) {
 func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) compositeRule {
 	rules := make(compositeRule, 0, len(visibility))
 	hasPrivateRule := false
+	hasPublicRule := false
 	hasNonPrivateRule := false
 	for _, v := range visibility {
 		ok, pkg, name := splitRule(v, currentPkg)
@@ -328,6 +329,7 @@ func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) c
 				isPrivateRule = true
 			case "public":
 				r = publicRule{}
+				hasPublicRule = true
 			}
 		} else {
 			switch name {
@@ -355,6 +357,11 @@ func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) c
 		return compositeRule{privateRule{}}
 	}
 
+	if hasPublicRule {
+		// Public overrides all other rules so just return it.
+		return compositeRule{publicRule{}}
+	}
+
 	return rules
 }
 
@@ -415,21 +422,21 @@ func visibilityRuleEnforcer(ctx TopDownMutatorContext) {
 			return
 		}
 
-		rule := effectiveVisibilityRules(ctx, depQualified)
+		rule := effectiveVisibilityRules(ctx.Config(), depQualified)
 		if rule != nil && !rule.matches(qualified) {
 			ctx.ModuleErrorf("depends on %s which is not visible to this module", depQualified)
 		}
 	})
 }
 
-func effectiveVisibilityRules(ctx BaseModuleContext, qualified qualifiedModuleName) compositeRule {
-	moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx)
+func effectiveVisibilityRules(config Config, qualified qualifiedModuleName) compositeRule {
+	moduleToVisibilityRule := moduleToVisibilityRuleMap(config)
 	value, ok := moduleToVisibilityRule.Load(qualified)
 	var rule compositeRule
 	if ok {
 		rule = value.(compositeRule)
 	} else {
-		rule = packageDefaultVisibility(ctx, qualified)
+		rule = packageDefaultVisibility(config, qualified)
 	}
 	return rule
 }
@@ -441,8 +448,8 @@ func createQualifiedModuleName(ctx BaseModuleContext) qualifiedModuleName {
 	return qualified
 }
 
-func packageDefaultVisibility(ctx BaseModuleContext, moduleId qualifiedModuleName) compositeRule {
-	moduleToVisibilityRule := moduleToVisibilityRuleMap(ctx)
+func packageDefaultVisibility(config Config, moduleId qualifiedModuleName) compositeRule {
+	moduleToVisibilityRule := moduleToVisibilityRuleMap(config)
 	packageQualifiedId := moduleId.getContainingPackageId()
 	for {
 		value, ok := moduleToVisibilityRule.Load(packageQualifiedId)
@@ -469,7 +476,7 @@ func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) []string {
 	dir := ctx.OtherModuleDir(module)
 	qualified := qualifiedModuleName{dir, moduleName}
 
-	rule := effectiveVisibilityRules(ctx, qualified)
+	rule := effectiveVisibilityRules(ctx.Config(), qualified)
 
 	return rule.Strings()
 }

android/visibility_test.go

@@ -1,15 +1,17 @@
 package android
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/google/blueprint"
 )
 
 var visibilityTests = []struct {
-	name           string
-	fs             map[string][]byte
-	expectedErrors []string
+	name                string
+	fs                  map[string][]byte
+	expectedErrors      []string
+	effectiveVisibility map[qualifiedModuleName][]string
 }{
 	{
 		name: "invalid visibility: empty list",
@@ -493,6 +495,9 @@ var visibilityTests = []struct {
 					deps: ["libexample"],
 				}`),
 		},
+		effectiveVisibility: map[qualifiedModuleName][]string{
+			qualifiedModuleName{pkg: "top", name: "libexample"}: {"//visibility:public"},
+		},
 	},
 	{
 		name: "//visibility:public mixed with other from different defaults 1",
@@ -903,13 +908,27 @@ var visibilityTests = []struct {
 func TestVisibility(t *testing.T) {
 	for _, test := range visibilityTests {
 		t.Run(test.name, func(t *testing.T) {
-			_, errs := testVisibility(buildDir, test.fs)
+			ctx, errs := testVisibility(buildDir, test.fs)
 
 			CheckErrorsAgainstExpectations(t, errs, test.expectedErrors)
+
+			if test.effectiveVisibility != nil {
+				checkEffectiveVisibility(t, ctx, test.effectiveVisibility)
+			}
 		})
 	}
 }
 
+func checkEffectiveVisibility(t *testing.T, ctx *TestContext, effectiveVisibility map[qualifiedModuleName][]string) {
+	for moduleName, expectedRules := range effectiveVisibility {
+		rule := effectiveVisibilityRules(ctx.config, moduleName)
+		stringRules := rule.Strings()
+		if !reflect.DeepEqual(expectedRules, stringRules) {
+			t.Errorf("effective rules mismatch: expected %q, found %q", expectedRules, stringRules)
+		}
+	}
+}
+
 func testVisibility(buildDir string, fs map[string][]byte) (*TestContext, []error) {
 
 	// Create a new config per test as visibility information is stored in the config.

sdk/sdk_test.go

@@ -111,8 +111,14 @@ func TestSnapshotVisibility(t *testing.T) {
 			sdk_version: "none",
 		}
 
+		java_defaults {
+			name: "java-defaults",
+			visibility: ["//other/bar"], 
+		}
+
 		java_library {
 			name: "mypublicjavalib",
+			defaults: ["java-defaults"],
       visibility: ["//visibility:public"],
 			srcs: ["Test.java"],
 			system_modules: "none",