+2 -364

cmd/glot/breaking.go

reviewed

··· 4 4 "context" 5 5 "encoding/json" 6 6 "fmt" 7 7 - "log/slog" 8 7 "reflect" 9 9 - "sort" 10 8 11 9 "github.com/bluesky-social/indigo/atproto/atdata" 12 10 "github.com/bluesky-social/indigo/atproto/lexicon" 13 11 "github.com/bluesky-social/indigo/atproto/syntax" 12 12 + "tangled.org/bnewbold.net/cobalt/cmd/glot/lexlint" 14 13 15 14 "github.com/urfave/cli/v3" 16 15 ) ··· 86 85 return err 87 86 } 88 87 89 89 - issues := breakingMaps(nsid, local.Defs, remote.Defs) 88 88 + issues := lexlint.BreakingChanges(&remote, &local) 90 89 91 90 if cmd.Bool("json") { 92 91 for _, iss := range issues { ··· 111 110 } 112 111 return nil 113 112 } 114 114 - 115 115 - func breakingMaps(nsid syntax.NSID, localMap, remoteMap map[string]lexicon.SchemaDef) []LintIssue { 116 116 - issues := []LintIssue{} 117 117 - 118 118 - // TODO: maybe only care about the intersection of keys, not union? 119 119 - keyMap := map[string]bool{} 120 120 - for k := range localMap { 121 121 - keyMap[k] = true 122 122 - } 123 123 - for k := range remoteMap { 124 124 - keyMap[k] = true 125 125 - } 126 126 - keys := []string{} 127 127 - for k := range keyMap { 128 128 - keys = append(keys, k) 129 129 - } 130 130 - sort.Strings(keys) 131 131 - 132 132 - for _, k := range keys { 133 133 - // NOTE: adding or removing an entire definition or sub-object doesn't break anything 134 134 - local, ok := localMap[k] 135 135 - if !ok { 136 136 - continue 137 137 - } 138 138 - remote, ok := remoteMap[k] 139 139 - if !ok { 140 140 - continue 141 141 - } 142 142 - 143 143 - nestIssues := breakingDefs(nsid, k, local, remote) 144 144 - if len(nestIssues) > 0 { 145 145 - issues = append(issues, nestIssues...) 146 146 - } 147 147 - } 148 148 - 149 149 - return issues 150 150 - } 151 151 - 152 152 - func breakingDefs(nsid syntax.NSID, name string, local, remote lexicon.SchemaDef) []LintIssue { 153 153 - issues := []LintIssue{} 154 154 - 155 155 - // TODO: in some situations this sort of change might actually be allowed? 156 156 - if reflect.TypeOf(local) != reflect.TypeOf(remote) { 157 157 - issues = append(issues, LintIssue{ 158 158 - NSID: nsid, 159 159 - LintLevel: "error", 160 160 - LintName: "type-change", 161 161 - LintDescription: "schema definition type changed", 162 162 - Message: fmt.Sprintf("schema type changed (%s): %T != %T", name, local, remote), 163 163 - }) 164 164 - return issues 165 165 - } 166 166 - 167 167 - switch l := local.Inner.(type) { 168 168 - case lexicon.SchemaRecord: 169 169 - slog.Debug("checking record", "name", name, "nsid", nsid) 170 170 - r := remote.Inner.(lexicon.SchemaRecord) 171 171 - if l.Key != r.Key { 172 172 - issues = append(issues, LintIssue{ 173 173 - NSID: nsid, 174 174 - LintLevel: "error", 175 175 - LintName: "record-key-type", 176 176 - LintDescription: "record key type changed", 177 177 - Message: fmt.Sprintf("schema type changed (%s): %s != %s", name, l.Key, r.Key), 178 178 - }) 179 179 - } 180 180 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: l.Record}, lexicon.SchemaDef{Inner: r.Record})...) 181 181 - case lexicon.SchemaQuery: 182 182 - r := remote.Inner.(lexicon.SchemaQuery) 183 183 - // TODO: situation where overall parameters added/removed, and required fields involved 184 184 - if l.Parameters != nil && r.Parameters != nil { 185 185 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Parameters}, lexicon.SchemaDef{Inner: *r.Parameters})...) 186 186 - } 187 187 - // TODO: situation where output requirement changes 188 188 - if l.Output != nil && r.Output != nil { 189 189 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Output}, lexicon.SchemaDef{Inner: *r.Output})...) 190 190 - } 191 191 - // TODO: do Errors matter? 192 192 - case lexicon.SchemaProcedure: 193 193 - r := remote.Inner.(lexicon.SchemaProcedure) 194 194 - // TODO: situation where overall parameters added/removed, and required fields involved 195 195 - if l.Parameters != nil && r.Parameters != nil { 196 196 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Parameters}, lexicon.SchemaDef{Inner: *r.Parameters})...) 197 197 - } 198 198 - // TODO: situation where output requirement changes 199 199 - if l.Input != nil && r.Input != nil { 200 200 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Input}, lexicon.SchemaDef{Inner: *r.Input})...) 201 201 - } 202 202 - // TODO: situation where output requirement changes 203 203 - if l.Output != nil && r.Output != nil { 204 204 - issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Output}, lexicon.SchemaDef{Inner: *r.Output})...) 205 205 - } 206 206 - // TODO: do Errors matter? 207 207 - // TODO: lexicon.SchemaSubscription (and SchemaMessage) 208 208 - // TODO: lexicon.SchemaPermissionSet (and SchemaPermission) 209 209 - case lexicon.SchemaBody: 210 210 - r := remote.Inner.(lexicon.SchemaBody) 211 211 - if l.Encoding != r.Encoding { 212 212 - issues = append(issues, LintIssue{ 213 213 - NSID: nsid, 214 214 - LintLevel: "error", 215 215 - LintName: "body-encoding", 216 216 - LintDescription: "API endpoint body content type (encoding) changed", 217 217 - Message: fmt.Sprintf("body encoding changed (%s): %s != %s", name, l.Encoding, r.Encoding), 218 218 - }) 219 219 - } 220 220 - if l.Schema != nil && r.Schema != nil { 221 221 - issues = append(issues, breakingDefs(nsid, name, *l.Schema, *r.Schema)...) 222 222 - } 223 223 - case lexicon.SchemaBoolean: 224 224 - r := remote.Inner.(lexicon.SchemaBoolean) 225 225 - // NOTE: default can change safely 226 226 - if !eqOptBool(l.Const, r.Const) { 227 227 - issues = append(issues, LintIssue{ 228 228 - NSID: nsid, 229 229 - LintLevel: "error", 230 230 - LintName: "const-value", 231 231 - LintDescription: "schema const value change", 232 232 - Message: fmt.Sprintf("const value changed (%s): %v != %v", name, l.Const, r.Const), 233 233 - }) 234 234 - } 235 235 - case lexicon.SchemaInteger: 236 236 - r := remote.Inner.(lexicon.SchemaInteger) 237 237 - // NOTE: default can change safely 238 238 - if !eqOptInt(l.Const, r.Const) { 239 239 - issues = append(issues, LintIssue{ 240 240 - NSID: nsid, 241 241 - LintLevel: "error", 242 242 - LintName: "const-value", 243 243 - LintDescription: "schema const value change", 244 244 - Message: fmt.Sprintf("const value changed (%s): %v != %v", name, l.Const, r.Const), 245 245 - }) 246 246 - } 247 247 - sort.Ints(l.Enum) 248 248 - sort.Ints(r.Enum) 249 249 - if !reflect.DeepEqual(l.Enum, r.Enum) { 250 250 - issues = append(issues, LintIssue{ 251 251 - NSID: nsid, 252 252 - LintLevel: "error", 253 253 - LintName: "enum-values", 254 254 - LintDescription: "schema enum values change", 255 255 - Message: fmt.Sprintf("integer enum value changed (%s)", name), 256 256 - }) 257 257 - } 258 258 - if !eqOptInt(l.Minimum, r.Minimum) || !eqOptInt(l.Maximum, r.Maximum) { 259 259 - issues = append(issues, LintIssue{ 260 260 - NSID: nsid, 261 261 - LintLevel: "warn", 262 262 - LintName: "integer-range", 263 263 - LintDescription: "schema min/max values change", 264 264 - Message: fmt.Sprintf("integer min/max values changed (%s)", name), 265 265 - }) 266 266 - } 267 267 - case lexicon.SchemaString: 268 268 - r := remote.Inner.(lexicon.SchemaString) 269 269 - // NOTE: default can change safely 270 270 - if !eqOptString(l.Const, r.Const) { 271 271 - issues = append(issues, LintIssue{ 272 272 - NSID: nsid, 273 273 - LintLevel: "error", 274 274 - LintName: "const-value", 275 275 - LintDescription: "schema const value change", 276 276 - Message: fmt.Sprintf("const value changed (%s)", name), 277 277 - }) 278 278 - } 279 279 - sort.Strings(l.Enum) 280 280 - sort.Strings(r.Enum) 281 281 - if !reflect.DeepEqual(l.Enum, r.Enum) { 282 282 - issues = append(issues, LintIssue{ 283 283 - NSID: nsid, 284 284 - LintLevel: "error", 285 285 - LintName: "enum-values", 286 286 - LintDescription: "schema enum values change", 287 287 - Message: fmt.Sprintf("string enum value changed (%s)", name), 288 288 - }) 289 289 - } 290 290 - // NOTE: known values can change safely 291 291 - if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) || !eqOptInt(l.MinGraphemes, r.MinGraphemes) || !eqOptInt(l.MaxGraphemes, r.MaxGraphemes) { 292 292 - issues = append(issues, LintIssue{ 293 293 - NSID: nsid, 294 294 - LintLevel: "warn", 295 295 - LintName: "string-length", 296 296 - LintDescription: "string min/max length change", 297 297 - Message: fmt.Sprintf("string min/max length change (%s)", name), 298 298 - }) 299 299 - } 300 300 - if !eqOptString(l.Format, r.Format) { 301 301 - issues = append(issues, LintIssue{ 302 302 - NSID: nsid, 303 303 - LintLevel: "error", 304 304 - LintName: "string-format", 305 305 - LintDescription: "string format change", 306 306 - Message: fmt.Sprintf("string format changed (%s)", name), 307 307 - }) 308 308 - } 309 309 - case lexicon.SchemaBytes: 310 310 - r := remote.Inner.(lexicon.SchemaBytes) 311 311 - if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) { 312 312 - issues = append(issues, LintIssue{ 313 313 - NSID: nsid, 314 314 - LintLevel: "warn", 315 315 - LintName: "bytes-length", 316 316 - LintDescription: "bytes min/max length change", 317 317 - Message: fmt.Sprintf("bytes min/max length change (%s)", name), 318 318 - }) 319 319 - } 320 320 - case lexicon.SchemaCIDLink: 321 321 - // pass 322 322 - case lexicon.SchemaArray: 323 323 - r := remote.Inner.(lexicon.SchemaArray) 324 324 - if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) { 325 325 - issues = append(issues, LintIssue{ 326 326 - NSID: nsid, 327 327 - LintLevel: "warn", 328 328 - LintName: "array-length", 329 329 - LintDescription: "array min/max length change", 330 330 - Message: fmt.Sprintf("array min/max length change (%s)", name), 331 331 - }) 332 332 - } 333 333 - issues = append(issues, breakingDefs(nsid, name, l.Items, r.Items)...) 334 334 - case lexicon.SchemaObject: 335 335 - r := remote.Inner.(lexicon.SchemaObject) 336 336 - sort.Strings(l.Required) 337 337 - sort.Strings(r.Required) 338 338 - if !reflect.DeepEqual(l.Required, r.Required) { 339 339 - issues = append(issues, LintIssue{ 340 340 - NSID: nsid, 341 341 - LintLevel: "error", 342 342 - LintName: "object-required", 343 343 - LintDescription: "change in which fields are required", 344 344 - Message: fmt.Sprintf("required fields change (%s)", name), 345 345 - }) 346 346 - } 347 347 - sort.Strings(l.Nullable) 348 348 - sort.Strings(r.Nullable) 349 349 - if !reflect.DeepEqual(l.Nullable, r.Nullable) { 350 350 - issues = append(issues, LintIssue{ 351 351 - NSID: nsid, 352 352 - LintLevel: "error", 353 353 - LintName: "object-nullable", 354 354 - LintDescription: "change in which fields are nullable", 355 355 - Message: fmt.Sprintf("nullable fields change (%s)", name), 356 356 - }) 357 357 - } 358 358 - issues = append(issues, breakingMaps(nsid, l.Properties, r.Properties)...) 359 359 - case lexicon.SchemaBlob: 360 360 - r := remote.Inner.(lexicon.SchemaBlob) 361 361 - sort.Strings(l.Accept) 362 362 - sort.Strings(r.Accept) 363 363 - if !reflect.DeepEqual(l.Accept, r.Accept) { 364 364 - // TODO: how strong of a warning should this be? 365 365 - issues = append(issues, LintIssue{ 366 366 - NSID: nsid, 367 367 - LintLevel: "warn", 368 368 - LintName: "blob-accept", 369 369 - LintDescription: "change in blob accept (content-type)", 370 370 - Message: fmt.Sprintf("blob accept change (%s)", name), 371 371 - }) 372 372 - } 373 373 - if !eqOptInt(l.MaxSize, r.MaxSize) { 374 374 - issues = append(issues, LintIssue{ 375 375 - NSID: nsid, 376 376 - LintLevel: "warn", 377 377 - LintName: "blob-size", 378 378 - LintDescription: "blob maximum size change", 379 379 - Message: fmt.Sprintf("blob max size change (%s)", name), 380 380 - }) 381 381 - } 382 382 - case lexicon.SchemaParams: 383 383 - r := remote.Inner.(lexicon.SchemaParams) 384 384 - sort.Strings(l.Required) 385 385 - sort.Strings(r.Required) 386 386 - if !reflect.DeepEqual(l.Required, r.Required) { 387 387 - issues = append(issues, LintIssue{ 388 388 - NSID: nsid, 389 389 - LintLevel: "error", 390 390 - LintName: "params-required", 391 391 - LintDescription: "change in which fields are required", 392 392 - Message: fmt.Sprintf("required fields change (%s)", name), 393 393 - }) 394 394 - } 395 395 - issues = append(issues, breakingMaps(nsid, l.Properties, r.Properties)...) 396 396 - case lexicon.SchemaToken: 397 397 - // pass 398 398 - case lexicon.SchemaRef: 399 399 - r := remote.Inner.(lexicon.SchemaRef) 400 400 - if l.Ref != r.Ref { 401 401 - // NOTE: if the underlying schemas are the same this could be ok in some situations 402 402 - issues = append(issues, LintIssue{ 403 403 - NSID: nsid, 404 404 - LintLevel: "warn", 405 405 - LintName: "ref-change", 406 406 - LintDescription: "change in referenced lexicon", 407 407 - Message: fmt.Sprintf("ref change (%s): %s != %s", name, l.Ref, r.Ref), 408 408 - }) 409 409 - } 410 410 - case lexicon.SchemaUnion: 411 411 - r := remote.Inner.(lexicon.SchemaUnion) 412 412 - if !eqOptBool(l.Closed, r.Closed) { 413 413 - // TODO: going from default to explicit should be ok... 414 414 - issues = append(issues, LintIssue{ 415 415 - NSID: nsid, 416 416 - LintLevel: "error", 417 417 - LintName: "union-open-closed", 418 418 - LintDescription: "can't change union between open and closed", 419 419 - Message: fmt.Sprintf("union open/closed type changed (%s)", name), 420 420 - }) 421 421 - } 422 422 - // TODO: closed union and refs change 423 423 - if l.Closed != nil && *l.Closed { 424 424 - sort.Strings(l.Refs) 425 425 - sort.Strings(r.Refs) 426 426 - if !reflect.DeepEqual(l.Refs, r.Refs) { 427 427 - issues = append(issues, LintIssue{ 428 428 - NSID: nsid, 429 429 - LintLevel: "error", 430 430 - LintName: "union-closed-refs", 431 431 - LintDescription: "closed unions can not have types (refs) change", 432 432 - Message: fmt.Sprintf("closed union types (refs) changed (%s)", name), 433 433 - }) 434 434 - } 435 435 - } 436 436 - case lexicon.SchemaUnknown: 437 437 - // pass 438 438 - default: 439 439 - slog.Warn("unhandled schema def type in breaking check", "type", reflect.TypeOf(local.Inner)) 440 440 - } 441 441 - 442 442 - return issues 443 443 - } 444 444 - 445 445 - // helper to check if two optional (pointer) integers are equal/consistent 446 446 - func eqOptInt(a, b *int) bool { 447 447 - if a == nil { 448 448 - return b == nil 449 449 - } 450 450 - if b == nil { 451 451 - return false 452 452 - } 453 453 - return *a == *b 454 454 - } 455 455 - 456 456 - func eqOptBool(a, b *bool) bool { 457 457 - if a == nil { 458 458 - return b == nil 459 459 - } 460 460 - if b == nil { 461 461 - return false 462 462 - } 463 463 - return *a == *b 464 464 - } 465 465 - 466 466 - func eqOptString(a, b *string) bool { 467 467 - if a == nil { 468 468 - return b == nil 469 469 - } 470 470 - if b == nil { 471 471 - return false 472 472 - } 473 473 - return *a == *b 474 474 - }

+376

cmd/glot/lexlint/breaking.go

reviewed

··· 1 1 + package lexlint 2 2 + 3 3 + import ( 4 4 + "fmt" 5 5 + "log/slog" 6 6 + "reflect" 7 7 + "sort" 8 8 + 9 9 + "github.com/bluesky-social/indigo/atproto/lexicon" 10 10 + "github.com/bluesky-social/indigo/atproto/syntax" 11 11 + ) 12 12 + 13 13 + func BreakingChanges(before, after *lexicon.SchemaFile) []LintIssue { 14 14 + return breakingMaps(syntax.NSID(before.ID), before.Defs, after.Defs) 15 15 + } 16 16 + 17 17 + func breakingMaps(nsid syntax.NSID, localMap, remoteMap map[string]lexicon.SchemaDef) []LintIssue { 18 18 + issues := []LintIssue{} 19 19 + 20 20 + // TODO: maybe only care about the intersection of keys, not union? 21 21 + keyMap := map[string]bool{} 22 22 + for k := range localMap { 23 23 + keyMap[k] = true 24 24 + } 25 25 + for k := range remoteMap { 26 26 + keyMap[k] = true 27 27 + } 28 28 + keys := []string{} 29 29 + for k := range keyMap { 30 30 + keys = append(keys, k) 31 31 + } 32 32 + sort.Strings(keys) 33 33 + 34 34 + for _, k := range keys { 35 35 + // NOTE: adding or removing an entire definition or sub-object doesn't break anything 36 36 + local, ok := localMap[k] 37 37 + if !ok { 38 38 + continue 39 39 + } 40 40 + remote, ok := remoteMap[k] 41 41 + if !ok { 42 42 + continue 43 43 + } 44 44 + 45 45 + nestIssues := breakingDefs(nsid, k, local, remote) 46 46 + if len(nestIssues) > 0 { 47 47 + issues = append(issues, nestIssues...) 48 48 + } 49 49 + } 50 50 + 51 51 + return issues 52 52 + } 53 53 + 54 54 + func breakingDefs(nsid syntax.NSID, name string, local, remote lexicon.SchemaDef) []LintIssue { 55 55 + issues := []LintIssue{} 56 56 + 57 57 + // TODO: in some situations this sort of change might actually be allowed? 58 58 + if reflect.TypeOf(local) != reflect.TypeOf(remote) { 59 59 + issues = append(issues, LintIssue{ 60 60 + NSID: nsid, 61 61 + LintLevel: "error", 62 62 + LintName: "type-change", 63 63 + LintDescription: "schema definition type changed", 64 64 + Message: fmt.Sprintf("schema type changed (%s): %T != %T", name, local, remote), 65 65 + }) 66 66 + return issues 67 67 + } 68 68 + 69 69 + switch l := local.Inner.(type) { 70 70 + case lexicon.SchemaRecord: 71 71 + slog.Debug("checking record", "name", name, "nsid", nsid) 72 72 + r := remote.Inner.(lexicon.SchemaRecord) 73 73 + if l.Key != r.Key { 74 74 + issues = append(issues, LintIssue{ 75 75 + NSID: nsid, 76 76 + LintLevel: "error", 77 77 + LintName: "record-key-type", 78 78 + LintDescription: "record key type changed", 79 79 + Message: fmt.Sprintf("schema type changed (%s): %s != %s", name, l.Key, r.Key), 80 80 + }) 81 81 + } 82 82 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: l.Record}, lexicon.SchemaDef{Inner: r.Record})...) 83 83 + case lexicon.SchemaQuery: 84 84 + r := remote.Inner.(lexicon.SchemaQuery) 85 85 + // TODO: situation where overall parameters added/removed, and required fields involved 86 86 + if l.Parameters != nil && r.Parameters != nil { 87 87 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Parameters}, lexicon.SchemaDef{Inner: *r.Parameters})...) 88 88 + } 89 89 + // TODO: situation where output requirement changes 90 90 + if l.Output != nil && r.Output != nil { 91 91 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Output}, lexicon.SchemaDef{Inner: *r.Output})...) 92 92 + } 93 93 + // TODO: do Errors matter? 94 94 + case lexicon.SchemaProcedure: 95 95 + r := remote.Inner.(lexicon.SchemaProcedure) 96 96 + // TODO: situation where overall parameters added/removed, and required fields involved 97 97 + if l.Parameters != nil && r.Parameters != nil { 98 98 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Parameters}, lexicon.SchemaDef{Inner: *r.Parameters})...) 99 99 + } 100 100 + // TODO: situation where output requirement changes 101 101 + if l.Input != nil && r.Input != nil { 102 102 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Input}, lexicon.SchemaDef{Inner: *r.Input})...) 103 103 + } 104 104 + // TODO: situation where output requirement changes 105 105 + if l.Output != nil && r.Output != nil { 106 106 + issues = append(issues, breakingDefs(nsid, name, lexicon.SchemaDef{Inner: *l.Output}, lexicon.SchemaDef{Inner: *r.Output})...) 107 107 + } 108 108 + // TODO: do Errors matter? 109 109 + // TODO: lexicon.SchemaSubscription (and SchemaMessage) 110 110 + // TODO: lexicon.SchemaPermissionSet (and SchemaPermission) 111 111 + case lexicon.SchemaBody: 112 112 + r := remote.Inner.(lexicon.SchemaBody) 113 113 + if l.Encoding != r.Encoding { 114 114 + issues = append(issues, LintIssue{ 115 115 + NSID: nsid, 116 116 + LintLevel: "error", 117 117 + LintName: "body-encoding", 118 118 + LintDescription: "API endpoint body content type (encoding) changed", 119 119 + Message: fmt.Sprintf("body encoding changed (%s): %s != %s", name, l.Encoding, r.Encoding), 120 120 + }) 121 121 + } 122 122 + if l.Schema != nil && r.Schema != nil { 123 123 + issues = append(issues, breakingDefs(nsid, name, *l.Schema, *r.Schema)...) 124 124 + } 125 125 + case lexicon.SchemaBoolean: 126 126 + r := remote.Inner.(lexicon.SchemaBoolean) 127 127 + // NOTE: default can change safely 128 128 + if !eqOptBool(l.Const, r.Const) { 129 129 + issues = append(issues, LintIssue{ 130 130 + NSID: nsid, 131 131 + LintLevel: "error", 132 132 + LintName: "const-value", 133 133 + LintDescription: "schema const value change", 134 134 + Message: fmt.Sprintf("const value changed (%s): %v != %v", name, l.Const, r.Const), 135 135 + }) 136 136 + } 137 137 + case lexicon.SchemaInteger: 138 138 + r := remote.Inner.(lexicon.SchemaInteger) 139 139 + // NOTE: default can change safely 140 140 + if !eqOptInt(l.Const, r.Const) { 141 141 + issues = append(issues, LintIssue{ 142 142 + NSID: nsid, 143 143 + LintLevel: "error", 144 144 + LintName: "const-value", 145 145 + LintDescription: "schema const value change", 146 146 + Message: fmt.Sprintf("const value changed (%s): %v != %v", name, l.Const, r.Const), 147 147 + }) 148 148 + } 149 149 + sort.Ints(l.Enum) 150 150 + sort.Ints(r.Enum) 151 151 + if !reflect.DeepEqual(l.Enum, r.Enum) { 152 152 + issues = append(issues, LintIssue{ 153 153 + NSID: nsid, 154 154 + LintLevel: "error", 155 155 + LintName: "enum-values", 156 156 + LintDescription: "schema enum values change", 157 157 + Message: fmt.Sprintf("integer enum value changed (%s)", name), 158 158 + }) 159 159 + } 160 160 + if !eqOptInt(l.Minimum, r.Minimum) || !eqOptInt(l.Maximum, r.Maximum) { 161 161 + issues = append(issues, LintIssue{ 162 162 + NSID: nsid, 163 163 + LintLevel: "warn", 164 164 + LintName: "integer-range", 165 165 + LintDescription: "schema min/max values change", 166 166 + Message: fmt.Sprintf("integer min/max values changed (%s)", name), 167 167 + }) 168 168 + } 169 169 + case lexicon.SchemaString: 170 170 + r := remote.Inner.(lexicon.SchemaString) 171 171 + // NOTE: default can change safely 172 172 + if !eqOptString(l.Const, r.Const) { 173 173 + issues = append(issues, LintIssue{ 174 174 + NSID: nsid, 175 175 + LintLevel: "error", 176 176 + LintName: "const-value", 177 177 + LintDescription: "schema const value change", 178 178 + Message: fmt.Sprintf("const value changed (%s)", name), 179 179 + }) 180 180 + } 181 181 + sort.Strings(l.Enum) 182 182 + sort.Strings(r.Enum) 183 183 + if !reflect.DeepEqual(l.Enum, r.Enum) { 184 184 + issues = append(issues, LintIssue{ 185 185 + NSID: nsid, 186 186 + LintLevel: "error", 187 187 + LintName: "enum-values", 188 188 + LintDescription: "schema enum values change", 189 189 + Message: fmt.Sprintf("string enum value changed (%s)", name), 190 190 + }) 191 191 + } 192 192 + // NOTE: known values can change safely 193 193 + if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) || !eqOptInt(l.MinGraphemes, r.MinGraphemes) || !eqOptInt(l.MaxGraphemes, r.MaxGraphemes) { 194 194 + issues = append(issues, LintIssue{ 195 195 + NSID: nsid, 196 196 + LintLevel: "warn", 197 197 + LintName: "string-length", 198 198 + LintDescription: "string min/max length change", 199 199 + Message: fmt.Sprintf("string min/max length change (%s)", name), 200 200 + }) 201 201 + } 202 202 + if !eqOptString(l.Format, r.Format) { 203 203 + issues = append(issues, LintIssue{ 204 204 + NSID: nsid, 205 205 + LintLevel: "error", 206 206 + LintName: "string-format", 207 207 + LintDescription: "string format change", 208 208 + Message: fmt.Sprintf("string format changed (%s)", name), 209 209 + }) 210 210 + } 211 211 + case lexicon.SchemaBytes: 212 212 + r := remote.Inner.(lexicon.SchemaBytes) 213 213 + if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) { 214 214 + issues = append(issues, LintIssue{ 215 215 + NSID: nsid, 216 216 + LintLevel: "warn", 217 217 + LintName: "bytes-length", 218 218 + LintDescription: "bytes min/max length change", 219 219 + Message: fmt.Sprintf("bytes min/max length change (%s)", name), 220 220 + }) 221 221 + } 222 222 + case lexicon.SchemaCIDLink: 223 223 + // pass 224 224 + case lexicon.SchemaArray: 225 225 + r := remote.Inner.(lexicon.SchemaArray) 226 226 + if !eqOptInt(l.MinLength, r.MinLength) || !eqOptInt(l.MaxLength, r.MaxLength) { 227 227 + issues = append(issues, LintIssue{ 228 228 + NSID: nsid, 229 229 + LintLevel: "warn", 230 230 + LintName: "array-length", 231 231 + LintDescription: "array min/max length change", 232 232 + Message: fmt.Sprintf("array min/max length change (%s)", name), 233 233 + }) 234 234 + } 235 235 + issues = append(issues, breakingDefs(nsid, name, l.Items, r.Items)...) 236 236 + case lexicon.SchemaObject: 237 237 + r := remote.Inner.(lexicon.SchemaObject) 238 238 + sort.Strings(l.Required) 239 239 + sort.Strings(r.Required) 240 240 + if !reflect.DeepEqual(l.Required, r.Required) { 241 241 + issues = append(issues, LintIssue{ 242 242 + NSID: nsid, 243 243 + LintLevel: "error", 244 244 + LintName: "object-required", 245 245 + LintDescription: "change in which fields are required", 246 246 + Message: fmt.Sprintf("required fields change (%s)", name), 247 247 + }) 248 248 + } 249 249 + sort.Strings(l.Nullable) 250 250 + sort.Strings(r.Nullable) 251 251 + if !reflect.DeepEqual(l.Nullable, r.Nullable) { 252 252 + issues = append(issues, LintIssue{ 253 253 + NSID: nsid, 254 254 + LintLevel: "error", 255 255 + LintName: "object-nullable", 256 256 + LintDescription: "change in which fields are nullable", 257 257 + Message: fmt.Sprintf("nullable fields change (%s)", name), 258 258 + }) 259 259 + } 260 260 + issues = append(issues, breakingMaps(nsid, l.Properties, r.Properties)...) 261 261 + case lexicon.SchemaBlob: 262 262 + r := remote.Inner.(lexicon.SchemaBlob) 263 263 + sort.Strings(l.Accept) 264 264 + sort.Strings(r.Accept) 265 265 + if !reflect.DeepEqual(l.Accept, r.Accept) { 266 266 + // TODO: how strong of a warning should this be? 267 267 + issues = append(issues, LintIssue{ 268 268 + NSID: nsid, 269 269 + LintLevel: "warn", 270 270 + LintName: "blob-accept", 271 271 + LintDescription: "change in blob accept (content-type)", 272 272 + Message: fmt.Sprintf("blob accept change (%s)", name), 273 273 + }) 274 274 + } 275 275 + if !eqOptInt(l.MaxSize, r.MaxSize) { 276 276 + issues = append(issues, LintIssue{ 277 277 + NSID: nsid, 278 278 + LintLevel: "warn", 279 279 + LintName: "blob-size", 280 280 + LintDescription: "blob maximum size change", 281 281 + Message: fmt.Sprintf("blob max size change (%s)", name), 282 282 + }) 283 283 + } 284 284 + case lexicon.SchemaParams: 285 285 + r := remote.Inner.(lexicon.SchemaParams) 286 286 + sort.Strings(l.Required) 287 287 + sort.Strings(r.Required) 288 288 + if !reflect.DeepEqual(l.Required, r.Required) { 289 289 + issues = append(issues, LintIssue{ 290 290 + NSID: nsid, 291 291 + LintLevel: "error", 292 292 + LintName: "params-required", 293 293 + LintDescription: "change in which fields are required", 294 294 + Message: fmt.Sprintf("required fields change (%s)", name), 295 295 + }) 296 296 + } 297 297 + issues = append(issues, breakingMaps(nsid, l.Properties, r.Properties)...) 298 298 + case lexicon.SchemaToken: 299 299 + // pass 300 300 + case lexicon.SchemaRef: 301 301 + r := remote.Inner.(lexicon.SchemaRef) 302 302 + if l.Ref != r.Ref { 303 303 + // NOTE: if the underlying schemas are the same this could be ok in some situations 304 304 + issues = append(issues, LintIssue{ 305 305 + NSID: nsid, 306 306 + LintLevel: "warn", 307 307 + LintName: "ref-change", 308 308 + LintDescription: "change in referenced lexicon", 309 309 + Message: fmt.Sprintf("ref change (%s): %s != %s", name, l.Ref, r.Ref), 310 310 + }) 311 311 + } 312 312 + case lexicon.SchemaUnion: 313 313 + r := remote.Inner.(lexicon.SchemaUnion) 314 314 + if !eqOptBool(l.Closed, r.Closed) { 315 315 + // TODO: going from default to explicit should be ok... 316 316 + issues = append(issues, LintIssue{ 317 317 + NSID: nsid, 318 318 + LintLevel: "error", 319 319 + LintName: "union-open-closed", 320 320 + LintDescription: "can't change union between open and closed", 321 321 + Message: fmt.Sprintf("union open/closed type changed (%s)", name), 322 322 + }) 323 323 + } 324 324 + // TODO: closed union and refs change 325 325 + if l.Closed != nil && *l.Closed { 326 326 + sort.Strings(l.Refs) 327 327 + sort.Strings(r.Refs) 328 328 + if !reflect.DeepEqual(l.Refs, r.Refs) { 329 329 + issues = append(issues, LintIssue{ 330 330 + NSID: nsid, 331 331 + LintLevel: "error", 332 332 + LintName: "union-closed-refs", 333 333 + LintDescription: "closed unions can not have types (refs) change", 334 334 + Message: fmt.Sprintf("closed union types (refs) changed (%s)", name), 335 335 + }) 336 336 + } 337 337 + } 338 338 + case lexicon.SchemaUnknown: 339 339 + // pass 340 340 + default: 341 341 + slog.Warn("unhandled schema def type in breaking check", "type", reflect.TypeOf(local.Inner)) 342 342 + } 343 343 + 344 344 + return issues 345 345 + } 346 346 + 347 347 + // helper to check if two optional (pointer) integers are equal/consistent 348 348 + func eqOptInt(a, b *int) bool { 349 349 + if a == nil { 350 350 + return b == nil 351 351 + } 352 352 + if b == nil { 353 353 + return false 354 354 + } 355 355 + return *a == *b 356 356 + } 357 357 + 358 358 + func eqOptBool(a, b *bool) bool { 359 359 + if a == nil { 360 360 + return b == nil 361 361 + } 362 362 + if b == nil { 363 363 + return false 364 364 + } 365 365 + return *a == *b 366 366 + } 367 367 + 368 368 + func eqOptString(a, b *string) bool { 369 369 + if a == nil { 370 370 + return b == nil 371 371 + } 372 372 + if b == nil { 373 373 + return false 374 374 + } 375 375 + return *a == *b 376 376 + }

+328

cmd/glot/lexlint/lint.go

reviewed

··· 1 1 + package lexlint 2 2 + 3 3 + import ( 4 4 + "errors" 5 5 + "fmt" 6 6 + "log/slog" 7 7 + "slices" 8 8 + 9 9 + "github.com/bluesky-social/indigo/atproto/lexicon" 10 10 + "github.com/bluesky-social/indigo/atproto/syntax" 11 11 + ) 12 12 + 13 13 + var ( 14 14 + // internal error used to set non-zero return code (but not print separately) 15 15 + ErrLintFailures = errors.New("linting issues detected") 16 16 + ) 17 17 + 18 18 + type LintIssue struct { 19 19 + FilePath string `json:"file-path,omitempty"` 20 20 + NSID syntax.NSID `json:"nsid,omitempty"` 21 21 + LintLevel string `json:"lint-level,omitempty"` 22 22 + LintName string `json:"lint-name,omitempty"` 23 23 + LintDescription string `json:"lint-description,omitempty"` 24 24 + Message string `json:"message,omitempty"` 25 25 + } 26 26 + 27 27 + func LintSchemaFile(sf *lexicon.SchemaFile) []LintIssue { 28 28 + issues := []LintIssue{} 29 29 + 30 30 + nsid, err := syntax.ParseNSID(sf.ID) 31 31 + if err != nil { 32 32 + issues = append(issues, LintIssue{ 33 33 + NSID: syntax.NSID(sf.ID), 34 34 + LintLevel: "error", 35 35 + LintName: "invalid-nsid", 36 36 + LintDescription: "schema file declares NSID with invalid syntax", 37 37 + Message: fmt.Sprintf("NSID string: %s", sf.ID), 38 38 + }) 39 39 + } 40 40 + if nsid == "" { 41 41 + nsid = syntax.NSID(sf.ID) 42 42 + } 43 43 + if sf.Lexicon != 1 { 44 44 + issues = append(issues, LintIssue{ 45 45 + NSID: nsid, 46 46 + LintLevel: "error", 47 47 + LintName: "lexicon-version", 48 48 + LintDescription: "unsupported Lexicon language version", 49 49 + Message: fmt.Sprintf("found version: %d", sf.Lexicon), 50 50 + }) 51 51 + return issues 52 52 + } 53 53 + 54 54 + for defname, def := range sf.Defs { 55 55 + defiss := lintSchemaDef(nsid, defname, def) 56 56 + if len(defiss) > 0 { 57 57 + issues = append(issues, defiss...) 58 58 + } 59 59 + } 60 60 + 61 61 + return issues 62 62 + } 63 63 + 64 64 + func lintSchemaDef(nsid syntax.NSID, defname string, def lexicon.SchemaDef) []LintIssue { 65 65 + issues := []LintIssue{} 66 66 + 67 67 + // missing description issue, in case it is needed 68 68 + missingDesc := func() LintIssue { 69 69 + return LintIssue{ 70 70 + NSID: nsid, 71 71 + LintLevel: "warn", 72 72 + LintName: "missing-primary-description", 73 73 + LintDescription: "primary types (record, query, procedure, subscription, permission-set) should include a description", 74 74 + Message: "primary type missing a description", 75 75 + } 76 76 + } 77 77 + 78 78 + if err := def.CheckSchema(); err != nil { 79 79 + issues = append(issues, LintIssue{ 80 80 + NSID: nsid, 81 81 + LintLevel: "error", 82 82 + LintName: "lexicon-schema", 83 83 + LintDescription: "basic structure schema checks (additional errors may be collapsed)", 84 84 + Message: err.Error(), 85 85 + }) 86 86 + } 87 87 + 88 88 + if err := CheckSchemaName(defname); err != nil { 89 89 + issues = append(issues, LintIssue{ 90 90 + NSID: nsid, 91 91 + LintLevel: "warn", 92 92 + LintName: "def-name-syntax", 93 93 + LintDescription: "definition name does not follow syntax guidance", 94 94 + Message: fmt.Sprintf("%s: %s", err.Error(), defname), 95 95 + }) 96 96 + } 97 97 + 98 98 + if nsid.Name() == "defs" && defname == "main" { 99 99 + issues = append(issues, LintIssue{ 100 100 + NSID: nsid, 101 101 + LintLevel: "warn", 102 102 + LintName: "defs-main-definition", 103 103 + LintDescription: "defs schemas should not have a 'main'", 104 104 + Message: "defs schemas should not have a 'main'", 105 105 + }) 106 106 + } 107 107 + 108 108 + switch def.Inner.(type) { 109 109 + case lexicon.SchemaRecord, lexicon.SchemaQuery, lexicon.SchemaProcedure, lexicon.SchemaSubscription, lexicon.SchemaPermissionSet: 110 110 + if defname != "main" { 111 111 + issues = append(issues, LintIssue{ 112 112 + NSID: nsid, 113 113 + LintLevel: "error", 114 114 + LintName: "non-main-primary", 115 115 + LintDescription: "primary types (record, query, procedure, subscription, permission-set) must be 'main' definition", 116 116 + Message: fmt.Sprintf("primary definition types must be 'main': %s", defname), 117 117 + }) 118 118 + } 119 119 + } 120 120 + 121 121 + switch v := def.Inner.(type) { 122 122 + case lexicon.SchemaRecord: 123 123 + if v.Description == nil || *v.Description == "" { 124 124 + issues = append(issues, missingDesc()) 125 125 + } 126 126 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: v.Record}) 127 127 + if len(reciss) > 0 { 128 128 + issues = append(issues, reciss...) 129 129 + } 130 130 + case lexicon.SchemaQuery: 131 131 + if v.Description == nil || *v.Description == "" { 132 132 + issues = append(issues, missingDesc()) 133 133 + } 134 134 + if v.Parameters != nil { 135 135 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 136 136 + if len(reciss) > 0 { 137 137 + issues = append(issues, reciss...) 138 138 + } 139 139 + } 140 140 + if v.Output == nil { 141 141 + issues = append(issues, LintIssue{ 142 142 + NSID: nsid, 143 143 + LintLevel: "warn", 144 144 + LintName: "endpoint-output-undefined", 145 145 + LintDescription: "API endpoints should define an output (even if empty)", 146 146 + Message: "missing output definition", 147 147 + }) 148 148 + } else { 149 149 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Output}) 150 150 + if len(reciss) > 0 { 151 151 + issues = append(issues, reciss...) 152 152 + } 153 153 + } 154 154 + // TODO: error names 155 155 + case lexicon.SchemaProcedure: 156 156 + if v.Description == nil || *v.Description == "" { 157 157 + issues = append(issues, missingDesc()) 158 158 + } 159 159 + if v.Parameters != nil { 160 160 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 161 161 + if len(reciss) > 0 { 162 162 + issues = append(issues, reciss...) 163 163 + } 164 164 + } 165 165 + if v.Input != nil { 166 166 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Input}) 167 167 + if len(reciss) > 0 { 168 168 + issues = append(issues, reciss...) 169 169 + } 170 170 + } 171 171 + if v.Output == nil { 172 172 + issues = append(issues, LintIssue{ 173 173 + NSID: nsid, 174 174 + LintLevel: "warn", 175 175 + LintName: "endpoint-output-undefined", 176 176 + LintDescription: "API endpoints should define an output (even if empty)", 177 177 + Message: "missing output definition", 178 178 + }) 179 179 + } else { 180 180 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Output}) 181 181 + if len(reciss) > 0 { 182 182 + issues = append(issues, reciss...) 183 183 + } 184 184 + } 185 185 + // TODO: error names 186 186 + case lexicon.SchemaSubscription: 187 187 + if v.Description == nil || *v.Description == "" { 188 188 + issues = append(issues, missingDesc()) 189 189 + } 190 190 + if v.Parameters != nil { 191 191 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 192 192 + if len(reciss) > 0 { 193 193 + issues = append(issues, reciss...) 194 194 + } 195 195 + } 196 196 + if v.Message != nil { 197 197 + // TODO: v.Message.Schema must only have local references (same file), and should have at least one defined 198 198 + reciss := lintSchemaRecursive(nsid, lexicon.SchemaDef{Inner: v.Message.Schema}) 199 199 + if len(reciss) > 0 { 200 200 + issues = append(issues, reciss...) 201 201 + } 202 202 + } else { 203 203 + issues = append(issues, LintIssue{ 204 204 + NSID: nsid, 205 205 + LintLevel: "warn", 206 206 + LintName: "subscription-no-messages", 207 207 + LintDescription: "no subscription message types defined", 208 208 + Message: "no subscription message types defined", 209 209 + }) 210 210 + } 211 211 + // TODO: at least one message type 212 212 + case lexicon.SchemaPermissionSet: 213 213 + if v.Title == nil || *v.Title == "" { 214 214 + issues = append(issues, LintIssue{ 215 215 + NSID: nsid, 216 216 + LintLevel: "warn", 217 217 + LintName: "permissionset-no-title", 218 218 + LintDescription: "permission sets should include a title", 219 219 + Message: "missing title", 220 220 + }) 221 221 + } 222 222 + // TODO: missing detail 223 223 + // TODO: translated descriptions? 224 224 + if len(v.Permissions) == 0 { 225 225 + issues = append(issues, LintIssue{ 226 226 + NSID: nsid, 227 227 + LintLevel: "warn", 228 228 + LintName: "permissionset-no-permissions", 229 229 + LintDescription: "permission sets should define at least one permission", 230 230 + Message: "empty permission set", 231 231 + }) 232 232 + } 233 233 + for _, perm := range v.Permissions { 234 234 + // TODO: any lints on permissions? 235 235 + _ = perm 236 236 + } 237 237 + case lexicon.SchemaPermission, lexicon.SchemaBoolean, lexicon.SchemaInteger, lexicon.SchemaString, lexicon.SchemaBytes, lexicon.SchemaCIDLink, lexicon.SchemaArray, lexicon.SchemaObject, lexicon.SchemaBlob, lexicon.SchemaToken, lexicon.SchemaRef, lexicon.SchemaUnion, lexicon.SchemaUnknown: 238 238 + reciss := lintSchemaRecursive(nsid, def) 239 239 + if len(reciss) > 0 { 240 240 + issues = append(issues, reciss...) 241 241 + } 242 242 + default: 243 243 + slog.Info("no lint rules for top-level schema definition type", "type", fmt.Sprintf("%T", def.Inner)) 244 244 + } 245 245 + return issues 246 246 + } 247 247 + 248 248 + func lintSchemaRecursive(nsid syntax.NSID, def lexicon.SchemaDef) []LintIssue { 249 249 + issues := []LintIssue{} 250 250 + 251 251 + switch v := def.Inner.(type) { 252 252 + case lexicon.SchemaPermission: 253 253 + // TODO: anything? 254 254 + case lexicon.SchemaBoolean: 255 255 + // TODO: default true 256 256 + // TODO: both default and const 257 257 + case lexicon.SchemaInteger: 258 258 + // TODO: both default and const 259 259 + case lexicon.SchemaString: 260 260 + // TODO: no format and no max length 261 261 + // TODO: format and length limits 262 262 + // TODO: grapheme limit set, and maxlen either too low or not set 263 263 + // TODO: very large max size 264 264 + // TODO: format=handle strings within an record type 265 265 + case lexicon.SchemaBytes: 266 266 + // TODO: very large max size 267 267 + case lexicon.SchemaCIDLink: 268 268 + // pass 269 269 + case lexicon.SchemaArray: 270 270 + reciss := lintSchemaRecursive(nsid, v.Items) 271 271 + if len(reciss) > 0 { 272 272 + issues = append(issues, reciss...) 273 273 + } 274 274 + case lexicon.SchemaObject: 275 275 + // NOTE: CheckSchema already verifies that nullable and required are valid against property keys 276 276 + for _, propdef := range v.Properties { 277 277 + reciss := lintSchemaRecursive(nsid, propdef) 278 278 + if len(reciss) > 0 { 279 279 + issues = append(issues, reciss...) 280 280 + } 281 281 + // TODO: property name syntax 282 282 + } 283 283 + for _, k := range v.Nullable { 284 284 + if !slices.Contains(v.Required, k) { 285 285 + issues = append(issues, LintIssue{ 286 286 + NSID: nsid, 287 287 + LintLevel: "warn", 288 288 + LintName: "nullable-and-optional", 289 289 + LintDescription: "object properties should not be both optional and nullable", 290 290 + Message: fmt.Sprintf("field is both nullable and optional: %s", k), 291 291 + }) 292 292 + } 293 293 + } 294 294 + case lexicon.SchemaBlob: 295 295 + // pass 296 296 + case lexicon.SchemaParams: 297 297 + // NOTE: CheckSchema already verifies that required are valid against property keys 298 298 + for _, propdef := range v.Properties { 299 299 + reciss := lintSchemaRecursive(nsid, propdef) 300 300 + if len(reciss) > 0 { 301 301 + issues = append(issues, reciss...) 302 302 + } 303 303 + // TODO: property name syntax 304 304 + } 305 305 + case lexicon.SchemaToken: 306 306 + // pass 307 307 + case lexicon.SchemaRef: 308 308 + // TODO: resolve? locally vs globally? 309 309 + case lexicon.SchemaUnion: 310 310 + // TODO: open vs closed? 311 311 + // TODO: check that refs actually resolve? 312 312 + case lexicon.SchemaUnknown: 313 313 + // pass 314 314 + case lexicon.SchemaBody: 315 315 + if v.Schema != nil { 316 316 + // NOTE: CheckSchema already verified that v.Schema is an object, ref, or union 317 317 + reciss := lintSchemaRecursive(nsid, *v.Schema) 318 318 + if len(reciss) > 0 { 319 319 + issues = append(issues, reciss...) 320 320 + } 321 321 + } 322 322 + // TODO: empty/invalid Encoding (mimetype) 323 323 + default: 324 324 + slog.Info("no lint rules for recursive schema type", "type", fmt.Sprintf("%T", def.Inner), "nsid", nsid) 325 325 + } 326 326 + 327 327 + return issues 328 328 + }

+21

cmd/glot/lexlint/syntax.go

reviewed

··· 1 1 + package lexlint 2 2 + 3 3 + import ( 4 4 + "errors" 5 5 + "regexp" 6 6 + ) 7 7 + 8 8 + var schemaNameRegex = regexp.MustCompile(`^[a-zA-Z]([a-zA-Z0-9]{0,255})?$`) 9 9 + 10 10 + func CheckSchemaName(raw string) error { 11 11 + if raw == "" { 12 12 + return errors.New("empty name") 13 13 + } 14 14 + if len(raw) > 255 { 15 15 + return errors.New("name is too long (255 chars max)") 16 16 + } 17 17 + if !schemaNameRegex.MatchString(raw) { 18 18 + return errors.New("name doesn't match recommended syntax/characters") 19 19 + } 20 20 + return nil 21 21 + }

+9 -348

cmd/glot/lint.go

reviewed

··· 4 4 "bytes" 5 5 "context" 6 6 "encoding/json" 7 7 - "errors" 8 7 "fmt" 9 8 "log/slog" 10 9 "os" 11 11 - "regexp" 12 12 - "slices" 13 10 14 11 "github.com/bluesky-social/indigo/atproto/lexicon" 15 12 "github.com/bluesky-social/indigo/atproto/syntax" 13 13 + "tangled.org/bnewbold.net/cobalt/cmd/glot/lexlint" 16 14 17 15 "github.com/urfave/cli/v3" 18 16 ) 19 17 20 18 var ( 21 21 - // internal error used to set non-zero return code (but not print separately) 22 22 - ErrLintFailures = errors.New("linting issues detected") 19 19 + ErrLintFailures = lexlint.ErrLintFailures 23 20 ) 24 21 25 22 var cmdLint = &cli.Command{ ··· 86 83 err = sf.FinishParse() 87 84 } 88 85 if err != nil { 89 89 - iss := LintIssue{ 86 86 + iss := lexlint.LintIssue{ 90 87 FilePath: p, 91 88 //NSID 92 89 LintLevel: "error", ··· 107 104 return ErrLintFailures 108 105 } 109 106 110 110 - issues := lintSchemaFile(p, sf) 107 107 + issues := lexlint.LintSchemaFile(&sf) 108 108 + for i := range issues { 109 109 + // add path as context 110 110 + issues[i].FilePath = p 111 111 + } 111 112 112 113 // check for unknown fields (more strict, as a lint/warning) 113 114 var unknownSF lexicon.SchemaFile 114 115 dec := json.NewDecoder(bytes.NewReader(b)) 115 116 dec.DisallowUnknownFields() 116 117 if err := dec.Decode(&unknownSF); err != nil { 117 117 - issues = append(issues, LintIssue{ 118 118 + issues = append(issues, lexlint.LintIssue{ 118 119 FilePath: p, 119 120 NSID: syntax.NSID(sf.ID), 120 121 LintLevel: "warn", ··· 147 148 } 148 149 return nil 149 150 } 150 150 - 151 151 - type LintIssue struct { 152 152 - FilePath string `json:"file-path,omitempty"` 153 153 - NSID syntax.NSID `json:"nsid,omitempty"` 154 154 - LintLevel string `json:"lint-level,omitempty"` 155 155 - LintName string `json:"lint-name,omitempty"` 156 156 - LintDescription string `json:"lint-description,omitempty"` 157 157 - Message string `json:"message,omitempty"` 158 158 - } 159 159 - 160 160 - var schemaNameRegex = regexp.MustCompile(`^[a-zA-Z]([a-zA-Z0-9]{0,255})?$`) 161 161 - 162 162 - func checkSchemaName(raw string) error { 163 163 - if raw == "" { 164 164 - return errors.New("empty name") 165 165 - } 166 166 - if len(raw) > 255 { 167 167 - return errors.New("name is too long (255 chars max)") 168 168 - } 169 169 - if !schemaNameRegex.MatchString(raw) { 170 170 - return errors.New("name doesn't match recommended syntax/characters") 171 171 - } 172 172 - return nil 173 173 - } 174 174 - 175 175 - func lintSchemaFile(p string, sf lexicon.SchemaFile) []LintIssue { 176 176 - issues := []LintIssue{} 177 177 - 178 178 - nsid, err := syntax.ParseNSID(sf.ID) 179 179 - if err != nil { 180 180 - issues = append(issues, LintIssue{ 181 181 - FilePath: p, 182 182 - NSID: syntax.NSID(sf.ID), 183 183 - LintLevel: "error", 184 184 - LintName: "invalid-nsid", 185 185 - LintDescription: "schema file declares NSID with invalid syntax", 186 186 - Message: fmt.Sprintf("NSID string: %s", sf.ID), 187 187 - }) 188 188 - } 189 189 - if nsid == "" { 190 190 - nsid = syntax.NSID(sf.ID) 191 191 - } 192 192 - if sf.Lexicon != 1 { 193 193 - issues = append(issues, LintIssue{ 194 194 - FilePath: p, 195 195 - NSID: nsid, 196 196 - LintLevel: "error", 197 197 - LintName: "lexicon-version", 198 198 - LintDescription: "unsupported Lexicon language version", 199 199 - Message: fmt.Sprintf("found version: %d", sf.Lexicon), 200 200 - }) 201 201 - return issues 202 202 - } 203 203 - 204 204 - for defname, def := range sf.Defs { 205 205 - defiss := lintSchemaDef(p, nsid, defname, def) 206 206 - if len(defiss) > 0 { 207 207 - issues = append(issues, defiss...) 208 208 - } 209 209 - } 210 210 - 211 211 - return issues 212 212 - } 213 213 - 214 214 - func lintSchemaDef(p string, nsid syntax.NSID, defname string, def lexicon.SchemaDef) []LintIssue { 215 215 - issues := []LintIssue{} 216 216 - 217 217 - // missing description issue, in case it is needed 218 218 - missingDesc := func() LintIssue { 219 219 - return LintIssue{ 220 220 - FilePath: p, 221 221 - NSID: nsid, 222 222 - LintLevel: "warn", 223 223 - LintName: "missing-primary-description", 224 224 - LintDescription: "primary types (record, query, procedure, subscription, permission-set) should include a description", 225 225 - Message: "primary type missing a description", 226 226 - } 227 227 - } 228 228 - 229 229 - if err := def.CheckSchema(); err != nil { 230 230 - issues = append(issues, LintIssue{ 231 231 - FilePath: p, 232 232 - NSID: nsid, 233 233 - LintLevel: "error", 234 234 - LintName: "lexicon-schema", 235 235 - LintDescription: "basic structure schema checks (additional errors may be collapsed)", 236 236 - Message: err.Error(), 237 237 - }) 238 238 - } 239 239 - 240 240 - if err := checkSchemaName(defname); err != nil { 241 241 - issues = append(issues, LintIssue{ 242 242 - FilePath: p, 243 243 - NSID: nsid, 244 244 - LintLevel: "warn", 245 245 - LintName: "def-name-syntax", 246 246 - LintDescription: "definition name does not follow syntax guidance", 247 247 - Message: fmt.Sprintf("%s: %s", err.Error(), defname), 248 248 - }) 249 249 - } 250 250 - 251 251 - if nsid.Name() == "defs" && defname == "main" { 252 252 - issues = append(issues, LintIssue{ 253 253 - FilePath: p, 254 254 - NSID: nsid, 255 255 - LintLevel: "warn", 256 256 - LintName: "defs-main-definition", 257 257 - LintDescription: "defs schemas should not have a 'main'", 258 258 - Message: "defs schemas should not have a 'main'", 259 259 - }) 260 260 - } 261 261 - 262 262 - switch def.Inner.(type) { 263 263 - case lexicon.SchemaRecord, lexicon.SchemaQuery, lexicon.SchemaProcedure, lexicon.SchemaSubscription, lexicon.SchemaPermissionSet: 264 264 - if defname != "main" { 265 265 - issues = append(issues, LintIssue{ 266 266 - FilePath: p, 267 267 - NSID: nsid, 268 268 - LintLevel: "error", 269 269 - LintName: "non-main-primary", 270 270 - LintDescription: "primary types (record, query, procedure, subscription, permission-set) must be 'main' definition", 271 271 - Message: fmt.Sprintf("primary definition types must be 'main': %s", defname), 272 272 - }) 273 273 - } 274 274 - } 275 275 - 276 276 - switch v := def.Inner.(type) { 277 277 - case lexicon.SchemaRecord: 278 278 - if v.Description == nil || *v.Description == "" { 279 279 - issues = append(issues, missingDesc()) 280 280 - } 281 281 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: v.Record}) 282 282 - if len(reciss) > 0 { 283 283 - issues = append(issues, reciss...) 284 284 - } 285 285 - case lexicon.SchemaQuery: 286 286 - if v.Description == nil || *v.Description == "" { 287 287 - issues = append(issues, missingDesc()) 288 288 - } 289 289 - if v.Parameters != nil { 290 290 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 291 291 - if len(reciss) > 0 { 292 292 - issues = append(issues, reciss...) 293 293 - } 294 294 - } 295 295 - if v.Output == nil { 296 296 - issues = append(issues, LintIssue{ 297 297 - FilePath: p, 298 298 - NSID: nsid, 299 299 - LintLevel: "warn", 300 300 - LintName: "endpoint-output-undefined", 301 301 - LintDescription: "API endpoints should define an output (even if empty)", 302 302 - Message: "missing output definition", 303 303 - }) 304 304 - } else { 305 305 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Output}) 306 306 - if len(reciss) > 0 { 307 307 - issues = append(issues, reciss...) 308 308 - } 309 309 - } 310 310 - // TODO: error names 311 311 - case lexicon.SchemaProcedure: 312 312 - if v.Description == nil || *v.Description == "" { 313 313 - issues = append(issues, missingDesc()) 314 314 - } 315 315 - if v.Parameters != nil { 316 316 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 317 317 - if len(reciss) > 0 { 318 318 - issues = append(issues, reciss...) 319 319 - } 320 320 - } 321 321 - if v.Input != nil { 322 322 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Input}) 323 323 - if len(reciss) > 0 { 324 324 - issues = append(issues, reciss...) 325 325 - } 326 326 - } 327 327 - if v.Output == nil { 328 328 - issues = append(issues, LintIssue{ 329 329 - FilePath: p, 330 330 - NSID: nsid, 331 331 - LintLevel: "warn", 332 332 - LintName: "endpoint-output-undefined", 333 333 - LintDescription: "API endpoints should define an output (even if empty)", 334 334 - Message: "missing output definition", 335 335 - }) 336 336 - } else { 337 337 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Output}) 338 338 - if len(reciss) > 0 { 339 339 - issues = append(issues, reciss...) 340 340 - } 341 341 - } 342 342 - // TODO: error names 343 343 - case lexicon.SchemaSubscription: 344 344 - if v.Description == nil || *v.Description == "" { 345 345 - issues = append(issues, missingDesc()) 346 346 - } 347 347 - if v.Parameters != nil { 348 348 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: *v.Parameters}) 349 349 - if len(reciss) > 0 { 350 350 - issues = append(issues, reciss...) 351 351 - } 352 352 - } 353 353 - if v.Message != nil { 354 354 - // TODO: v.Message.Schema must only have local references (same file), and should have at least one defined 355 355 - reciss := lintSchemaRecursive(p, nsid, lexicon.SchemaDef{Inner: v.Message.Schema}) 356 356 - if len(reciss) > 0 { 357 357 - issues = append(issues, reciss...) 358 358 - } 359 359 - } else { 360 360 - issues = append(issues, LintIssue{ 361 361 - FilePath: p, 362 362 - NSID: nsid, 363 363 - LintLevel: "warn", 364 364 - LintName: "subscription-no-messages", 365 365 - LintDescription: "no subscription message types defined", 366 366 - Message: "no subscription message types defined", 367 367 - }) 368 368 - } 369 369 - // TODO: at least one message type 370 370 - case lexicon.SchemaPermissionSet: 371 371 - if v.Title == nil || *v.Title == "" { 372 372 - issues = append(issues, LintIssue{ 373 373 - FilePath: p, 374 374 - NSID: nsid, 375 375 - LintLevel: "warn", 376 376 - LintName: "permissionset-no-title", 377 377 - LintDescription: "permission sets should include a title", 378 378 - Message: "missing title", 379 379 - }) 380 380 - } 381 381 - // TODO: missing detail 382 382 - // TODO: translated descriptions? 383 383 - if len(v.Permissions) == 0 { 384 384 - issues = append(issues, LintIssue{ 385 385 - FilePath: p, 386 386 - NSID: nsid, 387 387 - LintLevel: "warn", 388 388 - LintName: "permissionset-no-permissions", 389 389 - LintDescription: "permission sets should define at least one permission", 390 390 - Message: "empty permission set", 391 391 - }) 392 392 - } 393 393 - for _, perm := range v.Permissions { 394 394 - // TODO: any lints on permissions? 395 395 - _ = perm 396 396 - } 397 397 - case lexicon.SchemaPermission, lexicon.SchemaBoolean, lexicon.SchemaInteger, lexicon.SchemaString, lexicon.SchemaBytes, lexicon.SchemaCIDLink, lexicon.SchemaArray, lexicon.SchemaObject, lexicon.SchemaBlob, lexicon.SchemaToken, lexicon.SchemaRef, lexicon.SchemaUnion, lexicon.SchemaUnknown: 398 398 - reciss := lintSchemaRecursive(p, nsid, def) 399 399 - if len(reciss) > 0 { 400 400 - issues = append(issues, reciss...) 401 401 - } 402 402 - default: 403 403 - slog.Info("no lint rules for top-level schema definition type", "type", fmt.Sprintf("%T", def.Inner)) 404 404 - } 405 405 - return issues 406 406 - } 407 407 - 408 408 - func lintSchemaRecursive(p string, nsid syntax.NSID, def lexicon.SchemaDef) []LintIssue { 409 409 - issues := []LintIssue{} 410 410 - 411 411 - switch v := def.Inner.(type) { 412 412 - case lexicon.SchemaPermission: 413 413 - // TODO: anything? 414 414 - case lexicon.SchemaBoolean: 415 415 - // TODO: default true 416 416 - // TODO: both default and const 417 417 - case lexicon.SchemaInteger: 418 418 - // TODO: both default and const 419 419 - case lexicon.SchemaString: 420 420 - // TODO: no format and no max length 421 421 - // TODO: format and length limits 422 422 - // TODO: grapheme limit set, and maxlen either too low or not set 423 423 - // TODO: very large max size 424 424 - // TODO: format=handle strings within an record type 425 425 - case lexicon.SchemaBytes: 426 426 - // TODO: very large max size 427 427 - case lexicon.SchemaCIDLink: 428 428 - // pass 429 429 - case lexicon.SchemaArray: 430 430 - reciss := lintSchemaRecursive(p, nsid, v.Items) 431 431 - if len(reciss) > 0 { 432 432 - issues = append(issues, reciss...) 433 433 - } 434 434 - case lexicon.SchemaObject: 435 435 - // NOTE: CheckSchema already verifies that nullable and required are valid against property keys 436 436 - for _, propdef := range v.Properties { 437 437 - reciss := lintSchemaRecursive(p, nsid, propdef) 438 438 - if len(reciss) > 0 { 439 439 - issues = append(issues, reciss...) 440 440 - } 441 441 - // TODO: property name syntax 442 442 - } 443 443 - for _, k := range v.Nullable { 444 444 - if !slices.Contains(v.Required, k) { 445 445 - issues = append(issues, LintIssue{ 446 446 - FilePath: p, 447 447 - NSID: nsid, 448 448 - LintLevel: "warn", 449 449 - LintName: "nullable-and-optional", 450 450 - LintDescription: "object properties should not be both optional and nullable", 451 451 - Message: fmt.Sprintf("field is both nullable and optional: %s", k), 452 452 - }) 453 453 - } 454 454 - } 455 455 - case lexicon.SchemaBlob: 456 456 - // pass 457 457 - case lexicon.SchemaParams: 458 458 - // NOTE: CheckSchema already verifies that required are valid against property keys 459 459 - for _, propdef := range v.Properties { 460 460 - reciss := lintSchemaRecursive(p, nsid, propdef) 461 461 - if len(reciss) > 0 { 462 462 - issues = append(issues, reciss...) 463 463 - } 464 464 - // TODO: property name syntax 465 465 - } 466 466 - case lexicon.SchemaToken: 467 467 - // pass 468 468 - case lexicon.SchemaRef: 469 469 - // TODO: resolve? locally vs globally? 470 470 - case lexicon.SchemaUnion: 471 471 - // TODO: open vs closed? 472 472 - // TODO: check that refs actually resolve? 473 473 - case lexicon.SchemaUnknown: 474 474 - // pass 475 475 - case lexicon.SchemaBody: 476 476 - if v.Schema != nil { 477 477 - // NOTE: CheckSchema already verified that v.Schema is an object, ref, or union 478 478 - reciss := lintSchemaRecursive(p, nsid, *v.Schema) 479 479 - if len(reciss) > 0 { 480 480 - issues = append(issues, reciss...) 481 481 - } 482 482 - } 483 483 - // TODO: empty/invalid Encoding (mimetype) 484 484 - default: 485 485 - slog.Info("no lint rules for recursive schema type", "type", fmt.Sprintf("%T", def.Inner), "nsid", nsid) 486 486 - } 487 487 - 488 488 - return issues 489 489 - }