only trigger deprecation warnings for non-empty attributes (#5)

* only trigger deprecation warnings for non-empty attributes

A deprecation that targeted the use of an attributte would always trigger, now
we're only checking for deprecations on non-empty attributes.

* update dependencies

.github/workflows/lint.yaml

@@ -14,10 +14,10 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: '1.23.0'
+          go-version-file: go.mod
           cache: false # Let golangcilint handle caching
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         with:
-          version: v1.60
+          version: v1.61
           args: --timeout=4m

.github/workflows/test.yaml

@@ -14,7 +14,8 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: '1.23.0'
+          go-version-file: go.mod
+          cache: true
       - name: Run go tests
         run: |
           go test ./...

deprecation_test.go

@@ -37,7 +37,7 @@ func TestDeprecation(t *testing.T) {
 		counts = make(map[string]int)
 	)
 
-	dfn := func(
+	deprecationHandler := func(
 		_ context.Context, _ *newsdoc.Document,
 		deprecation revisor.Deprecation,
 		c revisor.DeprecationContext,
@@ -57,7 +57,7 @@ func TestDeprecation(t *testing.T) {
 	ctx := context.Background()
 
 	res, err := testValidator.ValidateDocument(ctx, &document,
-		revisor.WithDeprecationHandler(dfn))
+		revisor.WithDeprecationHandler(deprecationHandler))
 	must(t, err, "validate document")
 
 	t.Run("FoundDeprecations", func(t *testing.T) {

go.mod

@@ -1,16 +1,16 @@
 module github.com/ttab/revisor
 
-go 1.23.0
+go 1.23.2
 
 require (
 	github.com/IvanZagoskin/wkt v0.0.1
 	github.com/gobwas/glob v0.2.3
 	github.com/google/go-cmp v0.6.0
 	github.com/google/uuid v1.6.0
 	github.com/invopop/jsonschema v0.12.0
-	github.com/ttab/newsdoc v0.5.0
-	github.com/urfave/cli/v2 v2.27.2
-	golang.org/x/net v0.25.0
+	github.com/ttab/newsdoc v0.6.0
+	github.com/urfave/cli/v2 v2.27.4
+	golang.org/x/net v0.30.0
 )
 
 require (
@@ -20,6 +20,6 @@ require (
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/wk8/go-ordered-map/v2 v2.1.8 // indirect
-	github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 // indirect
+	github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -28,14 +28,22 @@ github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKs
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/ttab/newsdoc v0.5.0 h1:b4rvvQMiiaU0WUqYKPVP5p4kUlwN6X6HUcEn+isbDAQ=
 github.com/ttab/newsdoc v0.5.0/go.mod h1:a+D/6F/zNhIvNB7+JJMPx0HNm+146y68TCfVYo6H8WY=
+github.com/ttab/newsdoc v0.6.0 h1:xe6RYGBTXOJqXeIv7k/SeAMtocZlK+q4eRElw6jjVGQ=
+github.com/ttab/newsdoc v0.6.0/go.mod h1:9hqqHiu77aEp90HBNrHLWECjsfU5QI9hATpR0ZSGMgA=
 github.com/urfave/cli/v2 v2.27.2 h1:6e0H+AkS+zDckwPCUrZkKX38mRaau4nL2uipkJpbkcI=
 github.com/urfave/cli/v2 v2.27.2/go.mod h1:g0+79LmHHATl7DAcHO99smiR/T7uGLw84w8Y42x+4eM=
+github.com/urfave/cli/v2 v2.27.4 h1:o1owoI+02Eb+K107p27wEX9Bb8eqIoZCfLXloLUSWJ8=
+github.com/urfave/cli/v2 v2.27.4/go.mod h1:m4QzxcD2qpra4z7WhzEGn74WZLViBnMpb1ToCAKdGRQ=
 github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc=
 github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw=
 github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 h1:+qGGcbkzsfDQNPPe9UDgpxAWQrhbbBXOYJFQDq/dtJw=
 github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913/go.mod h1:4aEEwZQutDLsQv2Deui4iYQ6DWTxR14g6m8Wv88+Xqk=
+github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 h1:gEOO8jv9F4OT7lGCjxCBTO/36wtF6j2nSip77qHd4x4=
+github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM=
 golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac=
 golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM=
+golang.org/x/net v0.30.0 h1:AcW1SDZMkb8IpzCdQUaIq2sP4sZ4zw+55h6ynffypl4=
+golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

testdata/constraints/geo.json

@@ -83,6 +83,20 @@
               }
             }
           }
+        },
+        {
+          "match": {
+            "type": {"const":"core/place"}
+          },
+          "attributes": {
+            "role": {
+              "allowEmpty": true,
+              "deprecated": {
+                "label": "no-roles",
+                "doc": "Let's just skip roles"
+              }
+            }
+          }
         }
       ]
     }

testdata/results-deprecation/geo.enforced.json

@@ -18,6 +18,21 @@
     "error": "enforced deprecation \"absurdity\": Why did we ever think this was a good idea?",
     "enforcedDeprecation": true
   },
+  {
+    "entity": [
+      {
+        "refType": "attribute",
+        "name": "role"
+      },
+      {
+        "refType": "block",
+        "kind": "meta",
+        "type": "core/place"
+      }
+    ],
+    "error": "enforced deprecation \"no-roles\": Let's just skip roles",
+    "enforcedDeprecation": true
+  },
   {
     "entity": [
       {

testdata/results-deprecation/geo.json

@@ -29,6 +29,29 @@
       "value": "absurd"
     }
   },
+  {
+    "Deprecation": {
+      "label": "no-roles",
+      "doc": "Let's just skip roles"
+    },
+    "Context": {
+      "entity": {
+        "refType": "attribute",
+        "name": "role"
+      },
+      "block": {
+        "type": "core/place",
+        "data": {
+          "area": "POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10))",
+          "position": "POINT(12 15)",
+          "position_3d": "POINT Z (12 15 14)",
+          "road": "LINESTRING (30.123 10.15, 10.66 30.23, 40.23 40.66)"
+        },
+        "role": "absurd"
+      },
+      "value": "absurd"
+    }
+  },
   {
     "Deprecation": {
       "label": "3d-points",

validation.go

@@ -513,17 +513,20 @@ func validateDocumentAttributes(
 				})
 			}
 
-			res, err = checkDeprecation(
-				ctx, vCtx, res, d,
-				DeprecationContext{
-					Entity: &ref,
-					Value:  &value,
-				}, depr, check.Deprecated)
-			if err != nil {
-				return nil, err
-			}
-
 			if value != "" {
+				// As attributes always exist we only want to
+				// trigger a deprecation warning if they
+				// actually have a value.
+				res, err = checkDeprecation(
+					ctx, vCtx, res, d,
+					DeprecationContext{
+						Entity: &ref,
+						Value:  &value,
+					}, depr, check.Deprecated)
+				if err != nil {
+					return nil, err
+				}
+
 				vCtx.coll.CollectValue(ValueAnnotation{
 					Ref:   []EntityRef{ref},
 					Value: value,
@@ -808,17 +811,20 @@ func validateBlockAttributes(
 				})
 			}
 
-			res, err = checkDeprecation(
-				ctx, vCtx, res, doc, DeprecationContext{
-					Entity: &ref,
-					Block:  b,
-					Value:  &value,
-				}, check.Deprecated, depr)
-			if err != nil {
-				return nil, err
-			}
-
 			if value != "" {
+				// As attributes always exist we only want to
+				// trigger a deprecation warning if they
+				// actually have a value.
+				res, err = checkDeprecation(
+					ctx, vCtx, res, doc, DeprecationContext{
+						Entity: &ref,
+						Block:  b,
+						Value:  &value,
+					}, check.Deprecated, depr)
+				if err != nil {
+					return nil, err
+				}
+
 				vCtx.coll.CollectValue(ValueAnnotation{
 					Ref:        []EntityRef{ref},
 					Constraint: check,