Replicate blaze analyze's behavior with generated files.
1. If a library has all generated files, error.
2. If a library has a mix of literal and generated files, retain any unused deps, since they might have been manually added for the generated file.
There were also some major changes to the tests, since work that was previously done in a separate method was moved into setSources().
PiperOrigin-RevId: 242537197
diff --git a/ts_auto_deps/analyze/analyze.go b/ts_auto_deps/analyze/analyze.go
index b81c4b0..9e005cc 100644
--- a/ts_auto_deps/analyze/analyze.go
+++ b/ts_auto_deps/analyze/analyze.go
@@ -139,11 +139,16 @@
missingSources []string
// A map from the labels in the target's srcs to the Targets those
// labels refer.
- sources map[string]*appb.Target
+ sources map[string]*appb.Target
+ literalSourcePaths []string
+ generatedSourcePaths []string
}
// setSources sets the sources on t. It returns an error if one of the srcs of
-// t's rule isn't in loadedSrcs.
+// t's rule isn't in loadedSrcs. It also sorts the sources into literal and
+// generated sources, setting literalSourcePaths and generatedSourcePaths.
+// Returns an error if all the sources are generated - ts_auto_deps can't read the
+// import statements to determine deps.
func (t *resolvedTarget) setSources(loadedSrcs map[string]*appb.Target) error {
for _, label := range listAttribute(t.rule, "srcs") {
src := loadedSrcs[label]
@@ -151,6 +156,14 @@
return fmt.Errorf("no source found for label %s", label)
}
t.sources[label] = src
+ if src.GetType() == appb.Target_SOURCE_FILE {
+ t.literalSourcePaths = append(t.literalSourcePaths, labelToPath(label))
+ } else {
+ t.generatedSourcePaths = append(t.generatedSourcePaths, labelToPath(label))
+ }
+ }
+ if len(t.literalSourcePaths) == 0 && len(t.generatedSourcePaths) > 0 {
+ return fmt.Errorf("rule has generated sources - cannot determine dependencies")
}
return nil
}
@@ -165,37 +178,12 @@
return srcs, nil
}
-// literalSrcPaths returns the file paths of the non-generated sources of t.
-func (t *resolvedTarget) literalSrcPaths() ([]string, error) {
- srcs := listAttribute(t.rule, "srcs")
- if srcs == nil {
- return nil, fmt.Errorf("target %q missing \"srcs\" attribute", t.label)
- }
- var literalFilePaths []string
- for _, label := range listAttribute(t.rule, "srcs") {
- src := t.sources[label]
- if src == nil {
- return nil, fmt.Errorf("src %q has no associated target", label)
- }
- // There's no syntactic way to determine if a label is a source file
- // so check against the type of the relevant target
- if src.GetType() == appb.Target_SOURCE_FILE {
- literalFilePaths = append(literalFilePaths, labelToPath(label))
- }
- }
- return literalFilePaths, nil
-}
-
// getAllLiteralSrcPaths returns the file paths of all the non-generated sources
// of the targets.
func getAllLiteralSrcPaths(targets map[string]*resolvedTarget) ([]string, error) {
var allLiteralSrcPaths []string
for _, t := range targets {
- literalSrcPaths, err := t.literalSrcPaths()
- if err != nil {
- return nil, err
- }
- allLiteralSrcPaths = append(allLiteralSrcPaths, literalSrcPaths...)
+ allLiteralSrcPaths = append(allLiteralSrcPaths, t.literalSourcePaths...)
}
return allLiteralSrcPaths, nil
@@ -292,10 +280,7 @@
}
}
for _, t := range targets {
- srcs, err := t.literalSrcPaths()
- if err != nil {
- return nil, err
- }
+ srcs := t.literalSourcePaths
for _, src := range srcs {
v, ok := imports[src]
if ok {
@@ -570,7 +555,12 @@
// annotations. Unlike ts_declaration, there's no flag to remove them, so
// there's no need to report a warning.
default:
- report.UnnecessaryDependency = append(report.UnnecessaryDependency, label)
+ // The contents of generated files aren't visible, so ts_auto_deps can't discover
+ // the import statements/deps that they contain. To be safe, don't remove
+ // any unused deps, since they might be used by the generated file(s).
+ if len(target.generatedSourcePaths) == 0 {
+ report.UnnecessaryDependency = append(report.UnnecessaryDependency, label)
+ }
}
}
return report, nil
diff --git a/ts_auto_deps/analyze/analyze_test.go b/ts_auto_deps/analyze/analyze_test.go
index 140e025..ff529b8 100644
--- a/ts_auto_deps/analyze/analyze_test.go
+++ b/ts_auto_deps/analyze/analyze_test.go
@@ -7,8 +7,6 @@
"os"
"path/filepath"
"reflect"
- "sort"
- "strconv"
"strings"
"testing"
@@ -493,142 +491,15 @@
}
}
-func createResolvedTarget(srcs []string) *resolvedTarget {
- return &resolvedTarget{
- rule: &appb.Rule{
- Attribute: []*appb.Attribute{
- &appb.Attribute{
- Name: proto.String("srcs"),
- Type: appb.Attribute_STRING_LIST.Enum(),
- StringListValue: srcs,
- },
- },
- },
- sources: map[string]*appb.Target{
- "//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
- "//b:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
- "//b:generator": &appb.Target{Type: appb.Target_RULE.Enum()},
- "//b:wiz": &appb.Target{Type: appb.Target_RULE.Enum()},
- },
- }
-}
-
-func TestLiteralSrcPaths(t *testing.T) {
- tests := []struct {
- name string
- srcs []string
- err error
- expected []string
- }{
- {
- "OneLiteralSource",
- []string{"//a:file.ts"},
- nil,
- []string{"a/file.ts"},
- },
- {
- "MultipleLiteralSources",
- []string{"//a:file.ts", "//b:file.ts"},
- nil,
- []string{"a/file.ts", "b/file.ts"},
- },
- {
- "MultipleGeneratedSources",
- []string{"//b:generator", "//b:wiz"},
- nil,
- nil,
- },
- {
- "MixedSources",
- []string{"//a:file.ts", "//b:file.ts", "//b:generator", "//b:wiz"},
- nil,
- []string{"a/file.ts", "b/file.ts"},
- },
- {
- "MissingSource",
- []string{"//not/in/the/set/of/resolved:sources"},
- fmt.Errorf("src %q has no associated target", "//not/in/the/set/of/resolved:sources"),
- nil,
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- rt := createResolvedTarget(test.srcs)
- literalSrcPaths, err := rt.literalSrcPaths()
- if !reflect.DeepEqual(err, test.err) {
- t.Errorf("got err %q, expected %q", err, test.err)
- }
-
- if diff := pretty.Compare(literalSrcPaths, test.expected); diff != "" {
- t.Errorf("failed to get correct literal source paths: (-got, +want)\n%s", diff)
- }
- })
- }
-}
-
-func TestGetAllLiteralSrcPaths(t *testing.T) {
- tests := []struct {
- name string
- srcsLists [][]string
- err error
- expected []string
- }{
- {
- "OneTarget",
- [][]string{
- []string{"//a:file.ts", "//b:file.ts"},
- },
- nil,
- []string{"a/file.ts", "b/file.ts"},
- },
- {
- "MultipleTargets",
- [][]string{
- []string{"//a:file.ts"},
- []string{"//b:file.ts"},
- },
- nil,
- []string{"a/file.ts", "b/file.ts"},
- },
- {
- "MissingSource",
- [][]string{
- []string{"//not/in/the/set/of/resolved:sources"},
- },
- fmt.Errorf("src %q has no associated target", "//not/in/the/set/of/resolved:sources"),
- nil,
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- rts := make(map[string]*resolvedTarget)
- for i, srcs := range test.srcsLists {
- rts[strconv.Itoa(i)] = createResolvedTarget(srcs)
- }
- literalSrcPaths, err := getAllLiteralSrcPaths(rts)
- if !reflect.DeepEqual(err, test.err) {
- t.Errorf("got err %q, expected %q", err, test.err)
- }
-
- // getAllLiteralSrcPaths takes a map, so its output ordering isn't
- // deterministic
- sort.Strings(literalSrcPaths)
- if diff := pretty.Compare(literalSrcPaths, test.expected); diff != "" {
- t.Errorf("failed to get correct literal source paths: (-got, +want)\n%s", diff)
- }
- })
- }
-}
-
func TestSetSources(t *testing.T) {
tests := []struct {
- name string
- srcs []string
- loadedSrcs map[string]*appb.Target
- err error
- expected map[string]*appb.Target
+ name string
+ srcs []string
+ loadedSrcs map[string]*appb.Target
+ err error
+ expected map[string]*appb.Target
+ expectedLiteralPaths []string
+ expectedGeneratedPaths []string
}{
{
"NoSources",
@@ -636,6 +507,8 @@
nil,
nil,
nil,
+ nil,
+ nil,
},
{
"OneSource",
@@ -647,6 +520,8 @@
map[string]*appb.Target{
"//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
},
+ []string{"a/file.ts"},
+ nil,
},
{
"ExtraSources",
@@ -661,6 +536,54 @@
map[string]*appb.Target{
"//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
},
+ []string{"a/file.ts"},
+ nil,
+ },
+ {
+ "MultipleLiteralSources",
+ []string{"//a:file.ts", "//b:file.ts"},
+ map[string]*appb.Target{
+ "//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ },
+ nil,
+ map[string]*appb.Target{
+ "//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ },
+ []string{"a/file.ts", "b/file.ts"},
+ nil,
+ },
+ {
+ "MultipleGeneratedSources",
+ []string{"//b:generator", "//b:wiz"},
+ map[string]*appb.Target{
+ "//b:generator": &appb.Target{Type: appb.Target_RULE.Enum()},
+ "//b:wiz": &appb.Target{Type: appb.Target_RULE.Enum()},
+ },
+ fmt.Errorf("rule has generated sources - cannot determine dependencies"),
+ nil,
+ nil,
+ nil,
+ },
+ {
+ "MixedSources",
+ []string{"//a:file.ts", "//b:file.ts", "//b:generator", "//b:wiz"},
+ map[string]*appb.Target{
+ "//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:generator": &appb.Target{Type: appb.Target_RULE.Enum()},
+ "//b:wiz": &appb.Target{Type: appb.Target_RULE.Enum()},
+ },
+ nil,
+ map[string]*appb.Target{
+ "//a:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:file.ts": &appb.Target{Type: appb.Target_SOURCE_FILE.Enum()},
+ "//b:generator": &appb.Target{Type: appb.Target_RULE.Enum()},
+ "//b:wiz": &appb.Target{Type: appb.Target_RULE.Enum()},
+ },
+ []string{"a/file.ts", "b/file.ts"},
+ []string{"//b:generator", "//b:wiz"},
},
{
"MissingSources",
@@ -668,6 +591,8 @@
nil,
fmt.Errorf("no source found for label %s", "//a:file.ts"),
nil,
+ nil,
+ nil,
},
}
@@ -691,7 +616,7 @@
t.Errorf("got err %q, expected %q", err, test.err)
}
- if diff := pretty.Compare(rt.sources, test.expected); diff != "" {
+ if diff := pretty.Compare(rt.sources, test.expected); err == nil && diff != "" {
t.Errorf("failed to set correct sources: (-got, +want)\n%s", diff)
}
})
