Skip to content

Commit 00828d5

Browse files
brianquinlanbrianquinlan
authored
feat: Require that all imports have a version contraints (#2331)
Fixes #1989 This should not be landed before googleapis/google-cloud-rust#3396
1 parent 014b5f4 commit 00828d5

File tree

3 files changed

+44
-21
lines changed

3 files changed

+44
-21
lines changed

‎internal/sidekick/internal/dart/annotate.go‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ type annotateModel struct {
178178
state *api.APIState
179179
// The set of required imports (e.g. "package:google_cloud_type/type.dart" or
180180
// "package:http/http.dart as http") that have been calculated.
181+
//
182+
// The keys of this map are used to determine what imports to include
183+
// in the generated Dart code and what dependencies to include in
184+
// pubspec.yaml.
185+
//
186+
// Every import must have a corresponding entry in .sidekick.toml to specify
187+
// its version constraints.
181188
imports map[string]bool
182189
// The mapping from protobuf packages to Dart import statements.
183190
packageMapping map[string]string
@@ -302,8 +309,10 @@ func (annotate *annotateModel) annotateModel(options map[string]string) error {
302309
// Add the import for the google_cloud_gax package.
303310
annotate.imports[commonImport] = true
304311

305-
packageDependencies := calculateDependencies(annotate.imports, annotate.dependencyConstraints)
306-
312+
packageDependencies, err := calculateDependencies(annotate.imports, annotate.dependencyConstraints)
313+
if err != nil {
314+
return err
315+
}
307316
ann := &modelAnnotations{
308317
Parent: model,
309318
PackageName: packageName(model, packageNameOverride),
@@ -359,7 +368,7 @@ func calculateRequiredFields(model *api.API) map[string]*api.Field {
359368
}
360369

361370
// calculateDependencies calculates package dependencies based on `package:` imports.
362-
func calculateDependencies(imports map[string]bool, constraints map[string]string) []packageDependency {
371+
func calculateDependencies(imports map[string]bool, constraints map[string]string) ([]packageDependency, error) {
363372
deps := []packageDependency{}
364373

365374
for imp := range imports {
@@ -371,9 +380,7 @@ func calculateDependencies(imports map[string]bool, constraints map[string]strin
371380
}) {
372381
constraint := constraints[name]
373382
if len(constraint) == 0 {
374-
// TODO(https://github.com/googleapis/librarian/issues/1989):
375-
// Never emit "any" constraints.
376-
constraint = "any"
383+
return nil, fmt.Errorf("unknown version constraint for package %q (did you forget to add it to .sidekick.toml?)", name)
377384
}
378385
deps = append(deps, packageDependency{Name: name, Constraint: constraint})
379386
}
@@ -384,7 +391,7 @@ func calculateDependencies(imports map[string]bool, constraints map[string]strin
384391
return deps[i].Name < deps[j].Name
385392
})
386393

387-
return deps
394+
return deps, nil
388395
}
389396

390397
func calculateImports(imports map[string]bool) []string {

‎internal/sidekick/internal/dart/annotate_test.go‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,25 @@
1515
package dart
1616

1717
import (
18+
"maps"
1819
"testing"
1920

2021
"github.com/google/go-cmp/cmp"
2122
"github.com/googleapis/librarian/internal/sidekick/internal/api"
2223
"github.com/googleapis/librarian/internal/sidekick/internal/sample"
2324
)
2425

26+
var (
27+
requiredConfig = map[string]string{
28+
"package:google_cloud_gax": "^1.2.3",
29+
"package:http": "^4.5.6"}
30+
)
31+
2532
func TestAnnotateModel(t *testing.T) {
2633
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
2734
model.PackageName = "test"
2835
annotate := newAnnotateModel(model)
29-
err := annotate.annotateModel(map[string]string{})
36+
err := annotate.annotateModel(map[string]string{"package:google_cloud_gax": "^1.2.3"})
3037
if err != nil {
3138
t.Fatal(err)
3239
}
@@ -103,9 +110,9 @@ func TestAnnotateModel_Options(t *testing.T) {
103110
},
104111
},
105112
{
106-
map[string]string{"package:http": "1.2.0"},
113+
map[string]string{"google_cloud_gax": "^1.2.3", "package:http": "1.2.0"},
107114
func(t *testing.T, am *annotateModel) {
108-
if diff := cmp.Diff(map[string]string{"http": "1.2.0"}, am.dependencyConstraints); diff != "" {
115+
if diff := cmp.Diff(map[string]string{"google_cloud_gax": "^1.2.3", "http": "1.2.0"}, am.dependencyConstraints); diff != "" {
109116
t.Errorf("mismatch in annotateModel.dependencyConstraints (-want, +got)\n:%s", diff)
110117
}
111118
},
@@ -114,7 +121,9 @@ func TestAnnotateModel_Options(t *testing.T) {
114121

115122
for _, tt := range tests {
116123
annotate := newAnnotateModel(model)
117-
err := annotate.annotateModel(tt.options)
124+
options := maps.Clone(requiredConfig)
125+
maps.Copy(options, tt.options)
126+
err := annotate.annotateModel(maps.Clone(options))
118127
if err != nil {
119128
t.Fatal(err)
120129
}
@@ -140,7 +149,7 @@ func TestAnnotateMethod(t *testing.T) {
140149
)
141150
api.Validate(model)
142151
annotate := newAnnotateModel(model)
143-
err := annotate.annotateModel(map[string]string{})
152+
err := annotate.annotateModel(requiredConfig)
144153
if err != nil {
145154
t.Fatal(err)
146155
}
@@ -180,14 +189,17 @@ func TestCalculateDependencies(t *testing.T) {
180189
{name: "package imports", imports: []string{
181190
httpImport,
182191
"package:google_cloud_foo/foo.dart",
183-
}, want: []packageDependency{{Name: "google_cloud_foo", Constraint: "any"}, {Name: "http", Constraint: "^1.3.0"}}},
192+
}, want: []packageDependency{{Name: "google_cloud_foo", Constraint: "^1.2.3"}, {Name: "http", Constraint: "^1.3.0"}}},
184193
} {
185194
t.Run(test.name, func(t *testing.T) {
186195
deps := map[string]bool{}
187196
for _, imp := range test.imports {
188197
deps[imp] = true
189198
}
190-
got := calculateDependencies(deps, map[string]string{"http": "^1.3.0"})
199+
got, err := calculateDependencies(deps, map[string]string{"google_cloud_foo": "^1.2.3", "http": "^1.3.0"})
200+
if err != nil {
201+
t.Fatal(err)
202+
}
191203

192204
if diff := cmp.Diff(test.want, got); diff != "" {
193205
t.Errorf("mismatch in calculateDependencies (-want, +got)\n:%s", diff)

‎internal/sidekick/internal/dart/generate_test.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,16 @@ func TestFromProtobuf(t *testing.T) {
4646
"googleapis-root": path.Join(testdataDir, "googleapis"),
4747
},
4848
Codec: map[string]string{
49-
"copyright-year": "2025",
50-
"not-for-publication": "true",
51-
"version": "0.1.0",
52-
"skip-format": "true",
53-
"proto:google.protobuf": "package:google_cloud_protobuf/protobuf.dart",
54-
"proto:google.cloud.location": "package:google_cloud_location/location.dart",
49+
"copyright-year": "2025",
50+
"not-for-publication": "true",
51+
"version": "0.1.0",
52+
"skip-format": "true",
53+
"package:google_cloud_gax": "^1.2.3",
54+
"package:http": "^4.5.6",
55+
"package:google_cloud_location": "^7.8.9",
56+
"package:google_cloud_protobuf": "^0.1.2",
57+
"proto:google.protobuf": "package:google_cloud_protobuf/protobuf.dart",
58+
"proto:google.cloud.location": "package:google_cloud_location/location.dart",
5559
},
5660
}
5761
model, err := parser.CreateModel(cfg)
@@ -76,7 +80,7 @@ func TestFromProtobuf(t *testing.T) {
7680
func TestGeneratedFiles(t *testing.T) {
7781
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
7882
annotate := newAnnotateModel(model)
79-
annotate.annotateModel(map[string]string{})
83+
annotate.annotateModel(map[string]string{"package:google_cloud_gax": "^1.2.3", "package:http": "^4.5.6"})
8084
files := generatedFiles(model)
8185
if len(files) == 0 {
8286
t.Errorf("expected a non-empty list of template files from generatedFiles()")

0 commit comments

Comments
 (0)