Report visibility errors in both check and gather phases

Previously, errors that were found when splitting visibility rules were
only reported in the check phase and simply ignored during the gather
phase. That was because every visibility property that was processed in
the gather phase had already been checked in the check phase. However,
that is not strictly true as it has always been possible to add a
mutator between the check and gather phases that creates a module with
invalid visibility properties that will just be ignored. Fortunately,
that has not happened.

A follow up commit will add the capability to create modules after the
defaults have been applied which means the chances of invalid visibility
properties being ignored will increase.

This change makes both phases report any errors they find. It will not
have any impact on existing code as if any errors are reported in the
check phase then the build will exit before the gather phase. It will
prevent any invalid visibility errors from being ignored.

Bug: 155295806
Test: m nothing
Merged-In: I101fa6b03d2530b16e4394a9e466fead48be0ff0
Change-Id: I101fa6b03d2530b16e4394a9e466fead48be0ff0
(cherry picked from commit 0c83aba28e)
This commit is contained in:
Paul Duffin 2020-05-01 18:13:36 +01:00
parent abc9a647a2
commit b33d21bdaf
1 changed files with 17 additions and 14 deletions

View File

@ -246,14 +246,8 @@ func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility [
}
for _, v := range visibility {
ok, pkg, name := splitRule(v, currentPkg)
ok, pkg, name := splitRule(ctx, v, currentPkg, property)
if !ok {
// Visibility rule is invalid so ignore it. Keep going rather than aborting straight away to
// ensure all the rules on this module are checked.
ctx.PropertyErrorf(property,
"invalid visibility pattern %q must match"+
" //<package>:<module>, //<package> or :<module>",
v)
continue
}
@ -301,21 +295,24 @@ func visibilityRuleGatherer(ctx BottomUpMutatorContext) {
// Parse the visibility rules that control access to the module and store them by id
// for use when enforcing the rules.
if visibility := m.visibility(); visibility != nil {
rule := parseRules(ctx, currentPkg, m.visibility())
if rule != nil {
moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, rule)
primaryProperty := m.base().primaryVisibilityProperty
if primaryProperty != nil {
if visibility := primaryProperty.getStrings(); visibility != nil {
rule := parseRules(ctx, currentPkg, primaryProperty.getName(), visibility)
if rule != nil {
moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, rule)
}
}
}
}
func parseRules(ctx BaseModuleContext, currentPkg string, visibility []string) compositeRule {
func parseRules(ctx BaseModuleContext, currentPkg, property 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)
ok, pkg, name := splitRule(ctx, v, currentPkg, property)
if !ok {
continue
}
@ -376,10 +373,16 @@ func isAllowedFromOutsideVendor(pkg string, name string) bool {
return !isAncestor("vendor", pkg)
}
func splitRule(ruleExpression string, currentPkg string) (bool, string, string) {
func splitRule(ctx BaseModuleContext, ruleExpression string, currentPkg, property string) (bool, string, string) {
// Make sure that the rule is of the correct format.
matches := visibilityRuleRegexp.FindStringSubmatch(ruleExpression)
if ruleExpression == "" || matches == nil {
// Visibility rule is invalid so ignore it. Keep going rather than aborting straight away to
// ensure all the rules on this module are checked.
ctx.PropertyErrorf(property,
"invalid visibility pattern %q must match"+
" //<package>:<module>, //<package> or :<module>",
ruleExpression)
return false, "", ""
}