cleanup: improve typeddata naming, improve Node#dup tests, action u…

…pgrade (#3118) **What problem is this PR intended to solve?** - some minor C code cleanup - improved testing around `Node#dup` that should have been in #3117 - update CI to use `download-artifact` and `upload-artifact` v4 **Have you included adequate test coverage?** Yes. **Does this change affect the behavior of either the C or the Java implementations?** No functional changes.
sparklemotion · Feb 2, 2024 · b2548ef · b2548ef
2 parents 94a90a0 + 78c48b1
commit b2548ef
Show file tree

Hide file tree

Showing 17 changed files with 128 additions and 118 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -440,7 +440,7 @@ jobs:
           docker run --rm -v "$(pwd):/nokogiri" -w /nokogiri \
             ${DOCKER_IMAGE} \
             ./scripts/test-gem-build gems ruby
-      - uses: actions/upload-artifact@v3
+      - uses: actions/upload-artifact@v4
         with:
           name: generic-gem
           path: gems
@@ -462,7 +462,7 @@ jobs:
         with:
           ruby-version: "${{matrix.ruby}}"
           apt-get: "libxml2-dev libxslt1-dev pkg-config"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: generic-gem
           path: gems
@@ -483,7 +483,7 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "${{matrix.ruby}}"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: generic-gem
           path: gems
@@ -505,7 +505,7 @@ jobs:
         with:
           ruby-version: "${{matrix.ruby}}"
           mingw: "libxml2 libxslt"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: generic-gem
           path: gems
@@ -528,7 +528,7 @@ jobs:
         with:
           ruby-version: "${{matrix.ruby}}"
           mingw: "libxml2 libxslt"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: generic-gem
           path: gems
@@ -566,7 +566,7 @@ jobs:
           docker run --rm -v "$(pwd):/nokogiri" -w /nokogiri \
             ${DOCKER_IMAGE} \
             ./scripts/test-gem-build gems ${{matrix.plat}}
-      - uses: actions/upload-artifact@v3
+      - uses: actions/upload-artifact@v4
         with:
           name: "cruby-${{matrix.plat}}-gem"
           path: gems
@@ -583,7 +583,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: true
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x86-linux-gem
           path: gems
@@ -605,7 +605,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: true
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-aarch64-linux-gem
           path: gems
@@ -627,7 +627,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: true
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-arm-linux-gem
           path: gems
@@ -652,7 +652,7 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "${{matrix.ruby}}"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x86_64-linux-gem
           path: gems
@@ -669,7 +669,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: true
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x86_64-linux-gem
           path: gems
@@ -689,7 +689,7 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "${{matrix.ruby}}"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x86_64-darwin-gem
           path: gems
@@ -709,7 +709,7 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "${{matrix.ruby}}"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x64-mingw32-gem
           path: gems
@@ -730,7 +730,7 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "${{matrix.ruby}}"
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: cruby-x64-mingw-ucrt-gem
           path: gems
@@ -747,7 +747,7 @@ jobs:
         with:
           submodules: true
       - run: ./scripts/test-gem-build gems java
-      - uses: actions/upload-artifact@v3
+      - uses: actions/upload-artifact@v4
         with:
           name: jruby-gem
           path: gems
@@ -766,7 +766,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: true
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: jruby-gem
           path: gems

diff --git a/ext/nokogiri/html4_element_description.c b/ext/nokogiri/html4_element_description.c
@@ -1,7 +1,7 @@
 #include <nokogiri.h>
 
-static const rb_data_type_t html4_element_description_type = {
-  .wrap_struct_name = "Nokogiri::HTML4::ElementDescription",
+static const rb_data_type_t html_elem_desc_type = {
+  .wrap_struct_name = "htmlElemDesc",
   .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
@@ -20,7 +20,7 @@ required_attributes(VALUE self)
   VALUE list;
   int i;
 
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   list = rb_ary_new();
 
@@ -46,7 +46,7 @@ deprecated_attributes(VALUE self)
   VALUE list;
   int i;
 
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   list = rb_ary_new();
 
@@ -72,7 +72,7 @@ optional_attributes(VALUE self)
   VALUE list;
   int i;
 
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   list = rb_ary_new();
 
@@ -95,7 +95,7 @@ static VALUE
 default_sub_element(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->defaultsubelt) {
     return NOKOGIRI_STR_NEW2(description->defaultsubelt);
@@ -117,7 +117,7 @@ sub_elements(VALUE self)
   VALUE list;
   int i;
 
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   list = rb_ary_new();
 
@@ -140,7 +140,7 @@ static VALUE
 description(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   return NOKOGIRI_STR_NEW2(description->desc);
 }
@@ -155,7 +155,7 @@ static VALUE
 inline_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->isinline) { return Qtrue; }
   return Qfalse;
@@ -171,7 +171,7 @@ static VALUE
 deprecated_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->depr) { return Qtrue; }
   return Qfalse;
@@ -187,7 +187,7 @@ static VALUE
 empty_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->empty) { return Qtrue; }
   return Qfalse;
@@ -203,7 +203,7 @@ static VALUE
 save_end_tag_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->saveEndTag) { return Qtrue; }
   return Qfalse;
@@ -219,7 +219,7 @@ static VALUE
 implied_end_tag_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->endTag) { return Qtrue; }
   return Qfalse;
@@ -235,7 +235,7 @@ static VALUE
 implied_start_tag_eh(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (description->startTag) { return Qtrue; }
   return Qfalse;
@@ -251,7 +251,7 @@ static VALUE
 name(VALUE self)
 {
   const htmlElemDesc *description;
-  TypedData_Get_Struct(self, htmlElemDesc, &html4_element_description_type, description);
+  TypedData_Get_Struct(self, htmlElemDesc, &html_elem_desc_type, description);
 
   if (NULL == description->name) { return Qnil; }
   return NOKOGIRI_STR_NEW2(description->name);
@@ -271,7 +271,7 @@ get_description(VALUE klass, VALUE tag_name)
                                     );
 
   if (NULL == description) { return Qnil; }
-  return TypedData_Wrap_Struct(klass, &html4_element_description_type, DISCARD_CONST_QUAL(void *, description));
+  return TypedData_Wrap_Struct(klass, &html_elem_desc_type, DISCARD_CONST_QUAL(void *, description));
 }
 
 void

diff --git a/ext/nokogiri/xml_document.c b/ext/nokogiri/xml_document.c
@@ -133,8 +133,8 @@ memsize(const void *data)
   return memsize;
 }
 
-static const rb_data_type_t noko_xml_document_data_type = {
-  .wrap_struct_name = "Nokogiri::XML::Document",
+static const rb_data_type_t xml_doc_type = {
+  .wrap_struct_name = "xmlDoc",
   .function = {
     .dmark = mark,
     .dfree = dealloc,
@@ -146,7 +146,7 @@ static const rb_data_type_t noko_xml_document_data_type = {
 static VALUE
 _xml_document_alloc(VALUE klass)
 {
-  return TypedData_Wrap_Struct(klass, &noko_xml_document_data_type, NULL);
+  return TypedData_Wrap_Struct(klass, &xml_doc_type, NULL);
 }
 
 static void
@@ -707,7 +707,7 @@ xmlDocPtr
 noko_xml_document_unwrap(VALUE rb_document)
 {
   xmlDocPtr c_document;
-  TypedData_Get_Struct(rb_document, xmlDoc, &noko_xml_document_data_type, c_document);
+  TypedData_Get_Struct(rb_document, xmlDoc, &xml_doc_type, c_document);
   return c_document;
 }
 

diff --git a/ext/nokogiri/xml_element_content.c b/ext/nokogiri/xml_element_content.c
@@ -2,8 +2,8 @@
 
 VALUE cNokogiriXmlElementContent;
 
-static const rb_data_type_t element_content_data_type = {
-  .wrap_struct_name = "Nokogiri::XML::ElementContent",
+static const rb_data_type_t xml_element_content_type = {
+  .wrap_struct_name = "xmlElementContent",
   .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
@@ -17,7 +17,7 @@ static VALUE
 get_name(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   if (!elem->name) { return Qnil; }
   return NOKOGIRI_STR_NEW2(elem->name);
@@ -33,7 +33,7 @@ static VALUE
 get_type(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   return INT2NUM(elem->type);
 }
@@ -45,7 +45,7 @@ static VALUE
 get_c1(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   if (!elem->c1) { return Qnil; }
   return noko_xml_element_content_wrap(rb_iv_get(self, "@document"), elem->c1);
@@ -58,7 +58,7 @@ static VALUE
 get_c2(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   if (!elem->c2) { return Qnil; }
   return noko_xml_element_content_wrap(rb_iv_get(self, "@document"), elem->c2);
@@ -74,7 +74,7 @@ static VALUE
 get_occur(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   return INT2NUM(elem->ocur);
 }
@@ -89,7 +89,7 @@ static VALUE
 get_prefix(VALUE self)
 {
   xmlElementContentPtr elem;
-  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &xml_element_content_type, elem);
 
   if (!elem->prefix) { return Qnil; }
 
@@ -104,7 +104,7 @@ noko_xml_element_content_wrap(VALUE rb_document, xmlElementContentPtr c_element_
 {
   VALUE elem = TypedData_Wrap_Struct(
                  cNokogiriXmlElementContent,
-                 &element_content_data_type,
+                 &xml_element_content_type,
                  c_element_content
                );