Codebase list golang-golang-x-mod / 6e4e729
modfile: clean up SetRequire I started this change by expanding the documentation and tests for SetRequire. Unfortunately, the tests failed when the existing contents included duplicates of a module path: --- FAIL: TestSetRequire/existing_duplicate (0.00s) rule_test.go:1011: after Cleanup, len(Require) = 3; want 1 --- FAIL: TestSetRequire/existing_duplicate_multi (0.00s) rule_test.go:1011: after Cleanup, len(Require) = 3; want 1 So then I fixed the detected bug, by updating the Line entries (possibly marking them for removal) in the same loop that updates the Require entries. (We don't need to loop over f.Syntax.Stmt separately to remove deleted entries because f.Syntax.Cleanup already does that.) For golang/go#45965 Change-Id: I1b665c0832112de2c4273628f266dc3d966fefdd Reviewed-on: https://go-review.googlesource.com/c/mod/+/325230 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Bryan C. Mills 2 years ago
2 changed file(s) with 133 addition(s) and 65 deletion(s). Raw diff Collapse all Expand all
+52
-43
modfile/rule.go less more
869869 f.Require = append(f.Require, &Require{module.Version{Path: path, Version: vers}, indirect, line})
870870 }
871871
872 // SetRequire sets the requirements of f to contain exactly req, preserving
873 // any existing line comments contents (except for 'indirect' markings)
874 // for the module versions named in req.
872 // SetRequire updates the requirements of f to contain exactly req, preserving
873 // the existing block structure and line comment contents (except for 'indirect'
874 // markings) for the first requirement on each named module path.
875 //
876 // The Syntax field is ignored for the requirements in req.
877 //
878 // Any requirements not already present in the file are added to the block
879 // containing the last require line.
880 //
881 // The requirements in req must specify at most one distinct version for each
882 // module path.
883 //
884 // If any existing requirements may be removed, the caller should call Cleanup
885 // after all edits are complete.
875886 func (f *File) SetRequire(req []*Require) {
876887 need := make(map[string]string)
877888 indirect := make(map[string]bool)
878889 for _, r := range req {
890 if prev, dup := need[r.Mod.Path]; dup && prev != r.Mod.Version {
891 panic(fmt.Errorf("SetRequire called with conflicting versions for path %s (%s and %s)", r.Mod.Path, prev, r.Mod.Version))
892 }
879893 need[r.Mod.Path] = r.Mod.Version
880894 indirect[r.Mod.Path] = r.Indirect
881895 }
882896
897 // Update or delete the existing Require entries to preserve
898 // only the first for each module path in req.
883899 for _, r := range f.Require {
884 if v, ok := need[r.Mod.Path]; ok {
885 r.Mod.Version = v
886 r.Indirect = indirect[r.Mod.Path]
887 } else {
900 v, ok := need[r.Mod.Path]
901 if !ok {
902 // This line is redundant or its path is no longer required at all.
903 // Mark the requirement for deletion in Cleanup.
904 r.Syntax.Token = nil
888905 *r = Require{}
889906 }
890 }
891
892 var newStmts []Expr
893 for _, stmt := range f.Syntax.Stmt {
894 switch stmt := stmt.(type) {
895 case *LineBlock:
896 if len(stmt.Token) > 0 && stmt.Token[0] == "require" {
897 var newLines []*Line
898 for _, line := range stmt.Line {
899 if p, err := parseString(&line.Token[0]); err == nil && need[p] != "" {
900 if len(line.Comments.Before) == 1 && len(line.Comments.Before[0].Token) == 0 {
901 line.Comments.Before = line.Comments.Before[:0]
902 }
903 line.Token[1] = need[p]
904 delete(need, p)
905 setIndirect(line, indirect[p])
906 newLines = append(newLines, line)
907 }
907
908 r.Mod.Version = v
909 r.Indirect = indirect[r.Mod.Path]
910
911 if line := r.Syntax; line != nil && len(line.Token) > 0 {
912 if line.InBlock {
913 // If the line is preceded by an empty line, remove it; see
914 // https://golang.org/issue/33779.
915 if len(line.Comments.Before) == 1 && len(line.Comments.Before[0].Token) == 0 {
916 line.Comments.Before = line.Comments.Before[:0]
908917 }
909 if len(newLines) == 0 {
910 continue // drop stmt
918 if len(line.Token) >= 2 { // example.com v1.2.3
919 line.Token[1] = v
911920 }
912 stmt.Line = newLines
913 }
914
915 case *Line:
916 if len(stmt.Token) > 0 && stmt.Token[0] == "require" {
917 if p, err := parseString(&stmt.Token[1]); err == nil && need[p] != "" {
918 stmt.Token[2] = need[p]
919 delete(need, p)
920 setIndirect(stmt, indirect[p])
921 } else {
922 continue // drop stmt
921 } else {
922 if len(line.Token) >= 3 { // require example.com v1.2.3
923 line.Token[2] = v
923924 }
924925 }
925 }
926 newStmts = append(newStmts, stmt)
927 }
928 f.Syntax.Stmt = newStmts
929
926
927 setIndirect(line, r.Indirect)
928 }
929
930 delete(need, r.Mod.Path)
931 }
932
933 // Add new entries in the last block of the file for any paths that weren't
934 // already present.
935 //
936 // This step is nondeterministic, but the final result will be deterministic
937 // because we will sort the block.
930938 for path, vers := range need {
931939 f.AddNewRequire(path, vers, indirect[path])
932940 }
941
933942 f.SortBlocks()
934943 }
935944
+81
-22
modfile/rule_test.go less more
9191 },
9292 }
9393
94 type require struct {
95 path, vers string
96 indirect bool
97 }
98
9499 var setRequireTests = []struct {
95100 desc string
96101 in string
97 mods []struct {
98 path string
99 vers string
100 indirect bool
101 }
102 out string
102 mods []require
103 out string
103104 }{
104105 {
105106 `https://golang.org/issue/45932`,
110111 x.y/c v1.2.3
111112 )
112113 `,
113 []struct {
114 path string
115 vers string
116 indirect bool
117 }{
114 []require{
118115 {"x.y/a", "v1.2.3", false},
119116 {"x.y/b", "v1.2.3", false},
120117 {"x.y/c", "v1.2.3", false},
137134 x.y/d v1.2.3
138135 )
139136 `,
140 []struct {
141 path string
142 vers string
143 indirect bool
144 }{
137 []require{
145138 {"x.y/a", "v1.2.3", false},
146139 {"x.y/b", "v1.2.3", false},
147140 {"x.y/c", "v1.2.3", false},
167160 x.y/g v1.2.3 // indirect
168161 )
169162 `,
170 []struct {
171 path string
172 vers string
173 indirect bool
174 }{
163 []require{
175164 {"x.y/a", "v1.2.3", true},
176165 {"x.y/b", "v1.2.3", true},
177166 {"x.y/c", "v1.2.3", true},
190179 x.y/f v1.2.3 //indirect
191180 x.y/g v1.2.3 // indirect
192181 )
182 `,
183 },
184 {
185 `existing_multi`,
186 `module m
187 require x.y/a v1.2.3
188 require x.y/b v1.2.3
189 require x.y/c v1.0.0 // not v1.2.3!
190 require x.y/d v1.2.3 // comment kept
191 require x.y/e v1.2.3 // comment kept
192 require x.y/f v1.2.3 // indirect
193 require x.y/g v1.2.3 // indirect
194 `,
195 []require{
196 {"x.y/h", "v1.2.3", false},
197 {"x.y/a", "v1.2.3", false},
198 {"x.y/b", "v1.2.3", false},
199 {"x.y/c", "v1.2.3", false},
200 {"x.y/d", "v1.2.3", false},
201 {"x.y/e", "v1.2.3", true},
202 {"x.y/f", "v1.2.3", false},
203 {"x.y/g", "v1.2.3", false},
204 },
205 `module m
206 require x.y/a v1.2.3
207
208 require x.y/b v1.2.3
209
210 require x.y/c v1.2.3 // not v1.2.3!
211
212 require x.y/d v1.2.3 // comment kept
213
214 require x.y/e v1.2.3 // indirect; comment kept
215
216 require x.y/f v1.2.3
217
218 require (
219 x.y/g v1.2.3
220 x.y/h v1.2.3
221 )
222 `,
223 },
224 {
225 `existing_duplicate`,
226 `module m
227 require (
228 x.y/a v1.0.0 // zero
229 x.y/a v1.1.0 // one
230 x.y/a v1.2.3 // two
231 )
232 `,
233 []require{
234 {"x.y/a", "v1.2.3", true},
235 },
236 `module m
237 require x.y/a v1.2.3 // indirect; zero
238 `,
239 },
240 {
241 `existing_duplicate_multi`,
242 `module m
243 require x.y/a v1.0.0 // zero
244 require x.y/a v1.1.0 // one
245 require x.y/a v1.2.3 // two
246 `,
247 []require{
248 {"x.y/a", "v1.2.3", true},
249 },
250 `module m
251 require x.y/a v1.2.3 // indirect; zero
193252 `,
194253 },
195254 }
9411000
9421001 f := testEdit(t, tt.in, tt.out, true, func(f *File) error {
9431002 f.SetRequire(mods)
1003 f.Cleanup()
9441004 return nil
9451005 })
9461006
947 f.Cleanup()
9481007 if len(f.Require) != len(mods) {
9491008 t.Errorf("after Cleanup, len(Require) = %v; want %v", len(f.Require), len(mods))
9501009 }