From 5e13d64efabedb039ed4eb49abd4a9c9e848f72a Mon Sep 17 00:00:00 2001
From: Ryan Ly <rly@lbl.gov>
Date: Mon, 1 Jul 2024 12:33:29 -0400
Subject: [PATCH] Warn when unexpected keys in specs (#1134)

* Warn when unexpected keys in specs

* Update changelog
---
 CHANGELOG.md                                 |  7 ++++---
 src/hdmf/spec/spec.py                        |  4 ++++
 tests/unit/spec_tests/test_attribute_spec.py | 12 ++++++++++++
 tests/unit/spec_tests/test_dataset_spec.py   | 12 ++++++++++++
 tests/unit/spec_tests/test_group_spec.py     | 10 ++++++++++
 tests/unit/spec_tests/test_link_spec.py      | 12 ++++++++++++
 6 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f5db4918..74e048a19 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,12 +2,13 @@
 
 ## HDMF 3.14.2 (Upcoming)
 
-### Bug fixes
-- Fix iterator increment causing an extra +1 added after the end of completion. @CodyCBakerPhD [#1128](https://github.com/hdmf-dev/hdmf/pull/1128)
-
 ### Enhancements
+- Warn when unexpected keys are present in specs. @rly [#1134](https://github.com/hdmf-dev/hdmf/pull/1134)
 - Support appending to zarr arrays. @mavaylon1 [#1136](https://github.com/hdmf-dev/hdmf/pull/1136)
 
+### Bug fixes
+- Fix iterator increment causing an extra +1 added after the end of completion. @CodyCBakerPhD [#1128](https://github.com/hdmf-dev/hdmf/pull/1128)
+
 ## HDMF 3.14.1 (June 6, 2024)
 
 ### Bug fixes
diff --git a/src/hdmf/spec/spec.py b/src/hdmf/spec/spec.py
index 1a6e8d987..6d7d29e49 100644
--- a/src/hdmf/spec/spec.py
+++ b/src/hdmf/spec/spec.py
@@ -93,9 +93,13 @@ def build_spec(cls, spec_dict):
         vargs = cls.build_const_args(spec_dict)
         kwargs = dict()
         # iterate through the Spec docval and construct kwargs based on matching values in spec_dict
+        unused_vargs = list(vargs)
         for x in get_docval(cls.__init__):
             if x['name'] in vargs:
                 kwargs[x['name']] = vargs.get(x['name'])
+                unused_vargs.remove(x['name'])
+        if unused_vargs:
+            warn(f'Unexpected keys {unused_vargs} in spec {spec_dict}')
         return cls(**kwargs)
 
 
diff --git a/tests/unit/spec_tests/test_attribute_spec.py b/tests/unit/spec_tests/test_attribute_spec.py
index 15102e728..bac8e12a3 100644
--- a/tests/unit/spec_tests/test_attribute_spec.py
+++ b/tests/unit/spec_tests/test_attribute_spec.py
@@ -91,3 +91,15 @@ def test_build_spec_no_doc(self):
         msg = "AttributeSpec.__init__: missing argument 'doc'"
         with self.assertRaisesWith(TypeError, msg):
             AttributeSpec.build_spec(spec_dict)
+
+    def test_build_warn_extra_args(self):
+        spec_dict = {
+            'name': 'attribute1',
+            'doc': 'test attribute',
+            'dtype': 'int',
+            'quantity': '?',
+        }
+        msg = ("Unexpected keys ['quantity'] in spec {'name': 'attribute1', 'doc': 'test attribute', "
+               "'dtype': 'int', 'quantity': '?'}")
+        with self.assertWarnsWith(UserWarning, msg):
+            AttributeSpec.build_spec(spec_dict)
diff --git a/tests/unit/spec_tests/test_dataset_spec.py b/tests/unit/spec_tests/test_dataset_spec.py
index 0309aced4..008e8c6fc 100644
--- a/tests/unit/spec_tests/test_dataset_spec.py
+++ b/tests/unit/spec_tests/test_dataset_spec.py
@@ -245,3 +245,15 @@ def test_data_type_property_value(self):
                 group = GroupSpec('A group', name='group',
                                   data_type_inc=data_type_inc, data_type_def=data_type_def)
                 self.assertEqual(group.data_type, data_type)
+
+    def test_build_warn_extra_args(self):
+        spec_dict = {
+            'name': 'dataset1',
+            'doc': 'test dataset',
+            'dtype': 'int',
+            'required': True,
+        }
+        msg = ("Unexpected keys ['required'] in spec {'name': 'dataset1', 'doc': 'test dataset', "
+               "'dtype': 'int', 'required': True}")
+        with self.assertWarnsWith(UserWarning, msg):
+            DatasetSpec.build_spec(spec_dict)
diff --git a/tests/unit/spec_tests/test_group_spec.py b/tests/unit/spec_tests/test_group_spec.py
index 00a937538..31c00cfbb 100644
--- a/tests/unit/spec_tests/test_group_spec.py
+++ b/tests/unit/spec_tests/test_group_spec.py
@@ -314,6 +314,16 @@ def test_get_namespace_spec(self):
         expected = AttributeSpec('namespace', 'the namespace for the data type of this object', 'text', required=False)
         self.assertDictEqual(GroupSpec.get_namespace_spec(), expected)
 
+    def test_build_warn_extra_args(self):
+        spec_dict = {
+            'name': 'group1',
+            'doc': 'test group',
+            'required': True,
+        }
+        msg = "Unexpected keys ['required'] in spec {'name': 'group1', 'doc': 'test group', 'required': True}"
+        with self.assertWarnsWith(UserWarning, msg):
+            GroupSpec.build_spec(spec_dict)
+
 
 class TestNotAllowedConfig(TestCase):
 
diff --git a/tests/unit/spec_tests/test_link_spec.py b/tests/unit/spec_tests/test_link_spec.py
index e6c680b7c..38e10886b 100644
--- a/tests/unit/spec_tests/test_link_spec.py
+++ b/tests/unit/spec_tests/test_link_spec.py
@@ -67,3 +67,15 @@ def test_required_is_many(self):
                 )
                 self.assertEqual(spec.required, req)
                 self.assertEqual(spec.is_many(), many)
+
+    def test_build_warn_extra_args(self):
+        spec_dict = {
+            'name': 'link1',
+            'doc': 'test link',
+            'target_type': 'TestType',
+            'required': True,
+        }
+        msg = ("Unexpected keys ['required'] in spec {'name': 'link1', 'doc': 'test link', "
+               "'target_type': 'TestType', 'required': True}")
+        with self.assertWarnsWith(UserWarning, msg):
+            LinkSpec.build_spec(spec_dict)