Skip to content

Commit

Permalink
only trigger deprecation warnings for non-empty attributes
Browse files Browse the repository at this point in the history
A deprecation that targeted the use of an attributte would always trigger, now
we're only checking for deprecations on non-empty attributes.
  • Loading branch information
hugowetterberg committed Oct 11, 2024
1 parent c0474c7 commit 4756c43
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 22 deletions.
4 changes: 2 additions & 2 deletions deprecation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
14 changes: 14 additions & 0 deletions testdata/constraints/geo.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@
}
}
}
},
{
"match": {
"type": {"const":"core/place"}
},
"attributes": {
"role": {
"allowEmpty": true,
"deprecated": {
"label": "no-roles",
"doc": "Let's just skip roles"
}
}
}
}
]
}
Expand Down
15 changes: 15 additions & 0 deletions testdata/results-deprecation/geo.enforced.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
23 changes: 23 additions & 0 deletions testdata/results-deprecation/geo.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
46 changes: 26 additions & 20 deletions validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4756c43

Please sign in to comment.