Allow empty srcs (#138)

* Update java_index args processing

* Update test

* Allow existing rule srcs to be empty

* Allow scala rule to not have srcs

* Refactor .CanProvide to include from parameter

* Normalize relative labels for source provider canProvide

Author: pcj

README.md

@@ -472,7 +472,7 @@ func (p *bazelDepsProvider) loadFile(dir string, filename string, scope resolver
 }
 
 // CanProvide implements part of the resolver.SymbolProvider interface.
-func (p *bazelDepsProvider) CanProvide(dep *resolver.ImportLabel, knownRule func(from label.Label) (*rule.Rule, bool)) bool {
+func (p *bazelDepsProvider) CanProvide(dep *resolver.ImportLabel, knownRule func(from label.Label) (*rule.Rule, bool), from label.Label) bool {
 	if dep.Label.Repo == "bazel_deps" {
 		return true
 	}

language/scala/BUILD.bazel

@@ -38,6 +38,7 @@ go_library(
     deps = [
         "//build/stack/gazelle/scala/cache",
         "//build/stack/gazelle/scala/parse",
+        "//pkg/bazel",
         "//pkg/collections",
         "//pkg/glob",
         "//pkg/parser",

language/scala/coverage.go

@@ -1,6 +1,10 @@
 package scala
 
 import (
+	"fmt"
+	"sort"
+	"strings"
+
 	"github.com/stackb/scala-gazelle/pkg/procutil"
 )
 
@@ -11,26 +15,40 @@ type packageRuleCoverage struct {
 	// total represents the total number of rules in a package that we have a
 	// RuleProvider for.
 	total int
+	// kinds represents the kinds of rules covered, and how many each has
+	kinds map[string]int
 }
 
 func (sl *scalaLang) reportCoverage(printf func(format string, v ...any)) {
 	if !sl.existingScalaRuleCoverageFlagValue {
 		return
 	}
 
+	kindTotals := make(map[string]int)
+
 	var managed int
 	var total int
 	for _, pkg := range sl.packages {
 		managed += pkg.ruleCoverage.managed
 		total += pkg.ruleCoverage.total
+		for k, v := range pkg.ruleCoverage.kinds {
+			kindTotals[k] += v
+		}
+	}
+
+	kinds := make([]string, 0, len(kindTotals))
+	for kind := range kindTotals {
+		kinds = append(kinds, fmt.Sprintf("%s: %d", kind, kindTotals[kind]))
 	}
+	sort.Strings(kinds)
+	totals := strings.Join(kinds, ", ")
 
 	var percent float32
 	if total > 0 {
 		percent = float32(managed) / float32(total) * 100
 	}
 
 	if procutil.LookupBoolEnv(SCALA_GAZELLE_SHOW_COVERAGE, true) {
-		printf("scala-gazelle coverage is %0.1f%% (%d/%d)", percent, managed, total)
+		printf("scala-gazelle coverage is %0.1f%% (%d/%d) %s", percent, managed, total, totals)
 	}
 }

language/scala/existing_scala_rule.go

@@ -74,11 +74,15 @@ func (s *existingScalaRuleProvider) ProvideRule(cfg *scalarule.Config, pkg scala
 func (s *existingScalaRuleProvider) ResolveRule(cfg *scalarule.Config, pkg scalarule.Package, r *rule.Rule) scalarule.RuleProvider {
 	scalaRule, err := pkg.ParseRule(r, "srcs")
 	if err != nil {
-		if err == ErrRuleHasNoSrcs {
-			return nil // no need to print a warning
+		if err != ErrRuleHasNoSrcs {
+			log.Printf("skipping %s %s: unable to collect srcs: %v", r.Kind(), r.Name(), err)
+			return nil
 		}
-		log.Printf("skipping %s %s: unable to collect srcs: %v", r.Kind(), r.Name(), err)
-		return nil
+		// rule has no srcs.  This is OK for binary rules, sometimes they only
+		// have a main_class.
+		// if !s.isBinary {
+		// 	return nil // no need to print a warning
+		// }
 	}
 	if scalaRule == nil {
 		log.Panicln("scalaRule should not be nil!")
@@ -133,13 +137,15 @@ func (s *existingScalaRule) Resolve(rctx *scalarule.ResolveContext, importsRaw i
 	sc.Imports(imports, rctx.Rule, "deps", rctx.From)
 
 	commentsSrcs := rctx.Rule.AttrComments("srcs")
-	commentsSrcs.Before = nil
-	if sc.ShouldAnnotateImports() {
-		scalaconfig.AnnotateImports(imports, commentsSrcs, "import: ")
-	}
-	if sc.ShouldAnnotateRule() {
-		ruleComments := makeRuleComments(scalaRule.pb)
-		commentsSrcs.Before = append(commentsSrcs.Before, ruleComments...)
+	if commentsSrcs != nil {
+		commentsSrcs.Before = nil
+		if sc.ShouldAnnotateImports() {
+			scalaconfig.AnnotateImports(imports, commentsSrcs, "import: ")
+		}
+		if sc.ShouldAnnotateRule() {
+			ruleComments := makeRuleComments(scalaRule.pb)
+			commentsSrcs.Before = append(commentsSrcs.Before, ruleComments...)
+		}
 	}
 
 	if s.isLibrary {

language/scala/scala_package.go

@@ -14,6 +14,7 @@ import (
 	"github.com/bazelbuild/bazel-gazelle/rule"
 	"github.com/rs/zerolog"
 
+	sppb "github.com/stackb/scala-gazelle/build/stack/gazelle/scala/parse"
 	"github.com/stackb/scala-gazelle/pkg/glob"
 	"github.com/stackb/scala-gazelle/pkg/parser"
 	"github.com/stackb/scala-gazelle/pkg/resolver"
@@ -70,7 +71,9 @@ func newScalaPackage(
 		cfg:              cfg,
 		rules:            make(map[string]*rule.Rule),
 		resolveWork:      make([]func(), 0),
-		ruleCoverage:     &packageRuleCoverage{},
+		ruleCoverage: &packageRuleCoverage{
+			kinds: map[string]int{},
+		},
 	}
 
 	s.gen = s.generateRules(true)
@@ -144,7 +147,7 @@ func (s *scalaPackage) generateRules(enabled bool) []scalarule.RuleProvider {
 				// TOOD(pcj): consider adding .ContributesToCoverage or some
 				// other way of tracking which rules contribute to coverage
 				// calculation.
-				if provider.Name() != "scala_files" && provider.Name() != "scala_fileset" {
+				if ruleContributesToCoverage(provider.Name()) {
 					s.ruleCoverage.total += 1
 				}
 			} else {
@@ -158,6 +161,7 @@ func (s *scalaPackage) generateRules(enabled bool) []scalarule.RuleProvider {
 	for _, rc := range configuredRules {
 		if !rc.Enabled {
 			s.logger.Debug().Msgf("%s configuration not enabled, skipping rule generation", rc.Name)
+			log.Println(s.cfg.Rel(), ": NOT ENABLED RULE:", rc.Name)
 			continue
 		}
 
@@ -188,8 +192,13 @@ func (s *scalaPackage) generateRules(enabled bool) []scalarule.RuleProvider {
 					s.logger.Debug().Msgf("new resolved rule: %s %s", resolvedRule.Name(), resolvedRule.Kind())
 					rules = append(rules, resolvedRule)
 					// TODO: make this an API, not hardcode which rule names contribute to coverage
-					if resolvedRule.Name() != "scala_files" || resolvedRule.Name() != "scala_fileset" {
+					if ruleContributesToCoverage(resolvedRule.Name()) {
 						s.ruleCoverage.managed += 1
+						s.ruleCoverage.kinds[r.Kind()] += 1
+					}
+				} else {
+					if r.Kind() != "scala_files" {
+						log.Println(s.cfg.Rel(), ": SKIPPED RULE:", r.Kind(), r.Name())
 					}
 				}
 			}
@@ -223,33 +232,39 @@ func (s *scalaPackage) GeneratedRules() (rules []*rule.Rule) {
 }
 
 // ParseRule implements part of the scalarule.Package interface.
-func (s *scalaPackage) ParseRule(r *rule.Rule, attrName string) (scalarule.Rule, error) {
+func (s *scalaPackage) ParseRule(r *rule.Rule, attrName string) (scalaRule scalarule.Rule, err error) {
 	dir := filepath.Join(s.repoRootDir(), s.args.Rel)
 	srcs, err := glob.CollectFilenames(s.args.File, dir, r.Attr(attrName))
 	if err != nil {
 		return nil, err
 	}
+
 	scalaSrcs := make([]string, 0, len(srcs))
 	for _, src := range srcs {
 		if !strings.HasSuffix(src, ".scala") {
 			continue
 		}
 		scalaSrcs = append(scalaSrcs, src)
 	}
+	if len(scalaSrcs) == 0 {
+		err = ErrRuleHasNoSrcs
+	}
 
 	logger := s.logger.With().Str("kind", r.Kind()).Str("name", r.Name()).Logger()
 	logger.Debug().Msgf("%d scala files collected from %s", len(scalaSrcs), attrName)
 
-	if len(scalaSrcs) == 0 {
-		return nil, ErrRuleHasNoSrcs
-	}
-
 	from := s.cfg.MaybeRewrite(r.Kind(), label.Label{Pkg: s.args.Rel, Name: r.Name()})
 
-	rule, err := s.parser.ParseScalaRule(r.Kind(), from, dir, scalaSrcs...)
-	if err != nil {
-		logger.Warn().Err(err).Msg("parse error")
-		return nil, err
+	rule := &sppb.Rule{
+		Label: from.String(),
+		Kind:  r.Kind(),
+	}
+	if len(scalaSrcs) > 0 {
+		rule, err = s.parser.ParseScalaRule(r.Kind(), from, dir, scalaSrcs...)
+		if err != nil {
+			logger.Warn().Err(err).Msg("parse error")
+			return nil, err
+		}
 	}
 
 	ctx := &scalaRuleContext{
@@ -259,7 +274,9 @@ func (s *scalaPackage) ParseRule(r *rule.Rule, attrName string) (scalarule.Rule,
 		scope:       s.universe,
 	}
 
-	return newScalaRule(logger, ctx, rule), nil
+	scalaRule = newScalaRule(logger, ctx, rule)
+
+	return
 }
 
 // repoRootDir return the root directory of the repo.
@@ -307,3 +324,16 @@ func (s *scalaPackage) getProvidedRules(providers []scalarule.RuleProvider, shou
 func (p *scalaPackage) infof(format string, args ...any) string {
 	return fmt.Sprintf("INFO ["+p.args.Rel+"]: "+format, args...)
 }
+
+func ruleContributesToCoverage(name string) bool {
+	switch name {
+	case "scala_files":
+		return false
+	case "scala_fileset":
+		return false
+	case "semanticdb_index":
+		return false
+	default:
+		return true
+	}
+}

language/scala/scala_package_test.go

@@ -10,22 +10,22 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/mock"
 
+	sppb "github.com/stackb/scala-gazelle/build/stack/gazelle/scala/parse"
 	"github.com/stackb/scala-gazelle/pkg/resolver/mocks"
 	"github.com/stackb/scala-gazelle/pkg/scalaconfig"
-	"github.com/stackb/scala-gazelle/pkg/scalarule"
 )
 
 func TestScalaPackageParseRule(t *testing.T) {
-
 	for name, tc := range map[string]struct {
-		rule     *rule.Rule
-		attrName string
-		want     scalarule.Rule
-		wantErr  string
+		rule      *rule.Rule
+		attrName  string
+		wantFiles []*sppb.File
+		wantErr   string
 	}{
 		"degenerate": {
-			rule:    rule.NewRule("scala_library", "somelib"),
-			wantErr: "rule has no source files",
+			rule:      rule.NewRule("scala_library", "somelib"),
+			wantErr:   "rule has no source files",
+			wantFiles: nil,
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
@@ -58,7 +58,7 @@ func TestScalaPackageParseRule(t *testing.T) {
 			if diff := cmp.Diff(tc.wantErr, gotErr); diff != "" {
 				t.Errorf("error (-want +got):\n%s", diff)
 			}
-			if diff := cmp.Diff(tc.want, got); diff != "" {
+			if diff := cmp.Diff(tc.wantFiles, got.Files()); diff != "" {
 				t.Errorf("(-want +got):\n%s", diff)
 			}
 		})

language/scala/scala_rule.go

@@ -14,6 +14,7 @@ import (
 	"github.com/bazelbuild/bazel-gazelle/rule"
 	"github.com/rs/zerolog"
 
+	"github.com/stackb/scala-gazelle/pkg/bazel"
 	"github.com/stackb/scala-gazelle/pkg/collections"
 	"github.com/stackb/scala-gazelle/pkg/procutil"
 	"github.com/stackb/scala-gazelle/pkg/resolver"
@@ -55,11 +56,11 @@ type scalaRule struct {
 	exports map[string]resolve.ImportSpec
 }
 
-var bazel = "bazel"
+var defaultBazelExe = "bazel"
 
 func init() {
 	if bazelExe, ok := os.LookupEnv("SCALA_GAZELLE_BAZEL_EXECUTABLE"); ok {
-		bazel = bazelExe
+		defaultBazelExe = bazelExe
 	}
 }
 
@@ -430,10 +431,10 @@ func (r *scalaRule) putExport(imp string) {
 
 func (r *scalaRule) fixWildcardImport(rel, filename, wimp string) ([]string, error) {
 	fixer := wildcardimport.NewFixer(&wildcardimport.FixerOptions{
-		BazelExecutable: bazel,
+		BazelExecutable: defaultBazelExe,
 	})
 
-	bwd := wildcardimport.GetBuildWorkspaceDirectory()
+	bwd := bazel.GetBuildWorkspaceDirectory()
 
 	// sometimes the filename already include the relative path.
 	// FIXME(pcj): ensure pre-parsed files have the same filenames as non-preparsed ones.

pkg/bazel/BUILD.bazel

@@ -6,7 +6,10 @@ go_library(
     srcs = ["bazel.go"],
     importpath = "github.com/stackb/scala-gazelle/pkg/bazel",
     visibility = ["//visibility:public"],
-    deps = ["@io_bazel_rules_go//go/tools/bazel:go_default_library"],
+    deps = [
+        "//pkg/procutil",
+        "@io_bazel_rules_go//go/tools/bazel:go_default_library",
+    ],
 )
 
 package_filegroup(

pkg/bazel/bazel.go

@@ -1,11 +1,14 @@
 package bazel
 
 import (
+	"log"
 	"os"
+	"os/exec"
 	"path"
 	"regexp"
 
 	"github.com/bazelbuild/rules_go/go/tools/bazel"
+	"github.com/stackb/scala-gazelle/pkg/procutil"
 )
 
 // the name of an environment variable at runtime
@@ -14,10 +17,9 @@ const TEST_TMPDIR = "TEST_TMPDIR"
 var (
 	FindBinary   = bazel.FindBinary
 	ListRunfiles = bazel.ListRunfiles
+	nonWordRe    = regexp.MustCompile(`\W+`)
 )
 
-var nonWordRe = regexp.MustCompile(`\W+`)
-
 func CleanupLabel(in string) string {
 	return nonWordRe.ReplaceAllString(in, "_")
 }
@@ -30,3 +32,24 @@ func NewTmpDir(prefix string) (string, error) {
 	}
 	return os.MkdirTemp("", prefix)
 }
+
+func ExecCommand(bazelExe, command, label string) ([]byte, int, error) {
+	args := []string{command, label}
+
+	cmd := exec.Command(bazelExe, args...)
+	cmd.Dir = GetBuildWorkspaceDirectory()
+
+	log.Println("🧱", cmd.String())
+	output, err := cmd.CombinedOutput()
+	exitCode := procutil.CmdExitCode(cmd, err)
+
+	return output, exitCode, err
+}
+
+func GetBuildWorkspaceDirectory() string {
+	if bwd, ok := os.LookupEnv("BUILD_WORKSPACE_DIRECTORY"); ok {
+		return bwd
+	} else {
+		return "."
+	}
+}

pkg/provider/java_provider.go

@@ -91,7 +91,7 @@ func (p *JavaProvider) OnEnd() error {
 }
 
 // CanProvide implements part of the resolver.SymbolProvider interface.
-func (p *JavaProvider) CanProvide(dep *resolver.ImportLabel, expr build.Expr, knownRule func(from label.Label) (*rule.Rule, bool)) bool {
+func (p *JavaProvider) CanProvide(dep *resolver.ImportLabel, expr build.Expr, knownRule func(from label.Label) (*rule.Rule, bool), from label.Label) bool {
 	if _, ok := p.byLabel[dep.Label]; ok {
 		return true
 	}