Skip to content

Conversation

@nyurik
Copy link
Member

@nyurik nyurik commented Oct 5, 2025

To improve readability, and to avoid duplication in names, I tried to clean up the structs a bit. Note that these are even caught by Clippy:

TBD

  • in enum AttributeValue { U64(u64), I64(i64), Double(f64), String(String) }
    • Is it better to call I64/U64 as Signed/Unsigned, or maybe SignedInt/UnsignedInt?
  • in Signal, should it be multiplexer_indicator or multiplex_indicator ?
  • in SignalExtendedValueType, the IEEEfloat32Bit and IEEEfloat64Bit seems a bit too verbose... Would Float and Double suffice?
  • in ValueDescription, the value_descriptions: Vec<ValDescription> in Signal and EnvironmentVariable seems excessive - would descriptions suffice?

Breaking Changes

  • enum EnvType
    • EnvTypeFloatFloat
    • EnvTypeu64U64
    • EnvTypeDataData
  • struct SignalType
    • signal_type_namename
  • enum AccessNode
    • AccessNodeVectorXXXVectorXXX
    • AccessNodeNameName
  • enum AttributeValuedForObjectType
    • RawAttributeValueRaw
    • NetworkNodeAttributeValueNetworkNode
    • MessageDefinitionAttributeValueMessageDefinition
    • SignalAttributeValueSignal
    • EnvVariableAttributeValueEnvVariable
  • enum AttributeValueType
    • AttributeValueTypeIntInt
    • AttributeValueTypeHexHex
    • AttributeValueTypeFloatFloat
    • AttributeValueTypeStringString
    • AttributeValueTypeEnumEnum
  • struct ValDescription
    • aid
    • bdescription
  • enum AttributeValue
    • AttributeValueU64U64
    • AttributeValueI64I64
    • AttributeValueF64Double
    • AttributeValueCharStringString
  • struct ValueTable
    • value_table_namename
    • value_descriptionsdescription
  • enum Comment
    • Node: node_namename
    • Message: message_idid
    • Signal: signal_namename
    • EnvVar: env_var_namename
  • struct EnvironmentVariable
    • env_var_namename
    • env_var_typetyp
  • struct AttributeDefault
    • attribute_namename
    • attribute_valuevalue
  • struct AttributeValueForObject
    • attribute_namename
    • attribute_valuevalue
  • enum ValueDescription
    • Signal: signal_namename
    • EnvironmentVariable: env_var_namename
  • struct SignalGroups
    • signal_group_namename (?)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a breaking change to rename redundant names in the codebase to improve readability and comply with Rust Clippy lints. The changes systematically remove prefixes that duplicate the enum/struct type names.

  • Renamed enum variants to remove redundant type prefixes (e.g., EnvTypeFloatFloat)
  • Renamed struct fields to remove redundant type prefixes (e.g., signal_type_namename)
  • Updated field names to be more descriptive (e.g., aid, bdescription in ValDescription)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Oct 5, 2025

Codecov Report

❌ Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/parser.rs 96.59% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@nyurik nyurik force-pushed the naming branch 4 times, most recently from ff48fb4 to 262ae83 Compare October 5, 2025 07:31
To improve readability, and to avoid duplication in names, I tried to clean up the structs a bit. Note that these are even caught by Clippy:
* [clippy::struct_field_names](https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names)
* [clippy::enum_variant_names](https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names)

## Breaking Changes:

* `enum EnvType`
  * `EnvTypeFloat` → `Float`
  * `EnvTypeu64` → `U64`
  * `EnvTypeData` → `Data`
* `struct SignalType`
  * `signal_type_name` → `name`
* `enum AccessNode`
  * `AccessNodeVectorXXX` → `VectorXXX`
  * `AccessNodeName` → `Name`
* `enum AttributeValuedForObjectType`
  * `RawAttributeValue` → `Raw`
  * `NetworkNodeAttributeValue` → `NetworkNode`
  * `MessageDefinitionAttributeValue` → `MessageDefinition`
  * `SignalAttributeValue` → `Signal`
  * `EnvVariableAttributeValue` → `EnvVariable`
* `enum AttributeValueType`
  * `AttributeValueTypeInt` → `Int`
  * `AttributeValueTypeHex` → `Hex`
  * `AttributeValueTypeFloat` → `Float`
  * `AttributeValueTypeString` → `String`
  * `AttributeValueTypeEnum` → `Enum`
* `struct ValDescription`
  * `a` → `id`
  * `b` → `description`
* `enum AttributeValue`
  * `AttributeValueU64` → `U64`
  * `AttributeValueI64` → `I64`
  * `AttributeValueF64` → `Double`
  * `AttributeValueCharString` → `String`
* `struct ValueTable`
  * `value_table_name` → `name`
  * `value_descriptions` → `description`
* `enum Comment`
  * `Node`: `node_name` → `name`
  * `Message`: `message_id` → `id`
  * `Signal`: `signal_name` → `name`
  * `EnvVar`: `env_var_name` → `name`
* `struct EnvironmentVariable`
  * `env_var_name` → `name`
  * `env_var_type` → `typ`
* `struct AttributeDefault`
  * `attribute_name` → `name`
  * `attribute_value` → `value`
* `struct AttributeValueForObject`
  * `attribute_name` → `name`
  * `attribute_value` → `value`
* `enum ValueDescription`
  * `Signal`: `signal_name` → `name`
  * `EnvironmentVariable`: `env_var_name` → `name`
* `struct SignalGroups`
  * `signal_group_name` → `name` (?)
@nyurik nyurik force-pushed the naming branch 2 times, most recently from 9a60205 to e67179e Compare October 6, 2025 05:30
Copy link
Member

@tegimeki tegimeki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with interfaces changes on dependent crate. There are a lot of places to update but it's mostly a mechanical process and the resulting names are better and more succinct. I can update dbc-data to match once available on crates.io.

AttributeValueCharString(String),
U64(u64),
I64(i64),
Double(f64),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you choose Double instead of F64 to match the IEEE/DBC nomenclature? I could go either way but using the file format convention as you have seems slightly better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose it mainly because int and other C-style integer type naming was not specific (on purpose, by the C standard), whereas float/double has always had much stricter size guarantees. On the other hand, f64 can be mis-read as i64, hence the name. I am ok to rename it to f64 if you think its better though, for consistency

Comment on lines +400 to +414
id: f64,
description: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was always puzzled by a and b names, this is better

@nyurik nyurik changed the title BREAKING CHANGE: rename redundant names chore!: major struct and enum naming refactoring Oct 18, 2025
@nyurik nyurik merged commit ab9be16 into oxibus:main Oct 18, 2025
8 checks passed
@nyurik nyurik deleted the naming branch October 18, 2025 15:53
@nyurik nyurik mentioned this pull request Oct 18, 2025
nyurik added a commit that referenced this pull request Oct 20, 2025
## 🤖 New release

* `can-dbc`: 6.0.0 -> 7.0.0 (⚠ API breaking changes)

### ⚠ `can-dbc` breaking changes

```text
--- failure enum_struct_variant_field_added: pub enum struct variant field added ---

Description:
An enum's exhaustive struct variant has a new field, which has to be included when constructing or matching on this variant.
        ref: https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_struct_variant_field_added.ron

Failed in:
  field name of variant ValueDescription::Signal in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:544
  field name of variant ValueDescription::EnvironmentVariable in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:548
  field name of variant Comment::Node in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:458
  field id of variant Comment::Message in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:462
  field name of variant Comment::Signal in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:467
  field name of variant Comment::EnvVar in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:471

--- failure enum_struct_variant_field_missing: pub enum struct variant's field removed or renamed ---

Description:
A publicly-visible enum has a struct variant whose field is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_struct_variant_field_missing.ron

Failed in:
  field signal_name of variant ValueDescription::Signal, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:597
  field env_var_name of variant ValueDescription::EnvironmentVariable, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:601
  field node_name of variant Comment::Node, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:503
  field message_id of variant Comment::Message, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:507
  field signal_name of variant Comment::Signal, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:512
  field env_var_name of variant Comment::EnvVar, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:516

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_variant_added.ron

Failed in:
  variant AttributeValue:U64 in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:426
  variant AttributeValue:I64 in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:427
  variant AttributeValue:Double in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:428
  variant AttributeValue:String in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:429
  variant AttributeValuedForObjectType:Raw in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:394
  variant AttributeValuedForObjectType:NetworkNode in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:395
  variant AttributeValuedForObjectType:MessageDefinition in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:396
  variant AttributeValuedForObjectType:Signal in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:397
  variant AttributeValuedForObjectType:EnvVariable in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:398
  variant EnvType:Float in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:351
  variant EnvType:U64 in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:352
  variant EnvType:Data in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:353
  variant AccessNode:VectorXXX in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:381
  variant AccessNode:Name in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:382
  variant AttributeValueType:Int in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:404
  variant AttributeValueType:Hex in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:405
  variant AttributeValueType:Float in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:406
  variant AttributeValueType:String in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:407
  variant AttributeValueType:Enum in /tmp/.tmpLGhgsa/can-dbc/src/lib.rs:408

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_variant_missing.ron

Failed in:
  variant AttributeValue::AttributeValueU64, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:468
  variant AttributeValue::AttributeValueI64, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:469
  variant AttributeValue::AttributeValueF64, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:470
  variant AttributeValue::AttributeValueCharString, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:471
  variant AttributeValueType::AttributeValueTypeInt, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:444
  variant AttributeValueType::AttributeValueTypeHex, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:445
  variant AttributeValueType::AttributeValueTypeFloat, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:446
  variant AttributeValueType::AttributeValueTypeString, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:447
  variant AttributeValueType::AttributeValueTypeEnum, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:448
  variant AttributeValuedForObjectType::RawAttributeValue, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:434
  variant AttributeValuedForObjectType::NetworkNodeAttributeValue, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:435
  variant AttributeValuedForObjectType::MessageDefinitionAttributeValue, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:436
  variant AttributeValuedForObjectType::SignalAttributeValue, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:437
  variant AttributeValuedForObjectType::EnvVariableAttributeValue, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:438
  variant EnvType::EnvTypeFloat, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:387
  variant EnvType::EnvTypeu64, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:388
  variant EnvType::EnvTypeData, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:389
  variant AccessNode::AccessNodeVectorXXX, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:420
  variant AccessNode::AccessNodeName, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:421

--- failure feature_missing: package feature removed or renamed ---

Description:
A feature has been removed from this package's Cargo.toml. This will break downstream crates which enable that feature.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#cargo-feature-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/feature_missing.ron

Failed in:
  feature serde_derive in the package's Cargo.toml

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/inherent_method_missing.ron

Failed in:
  Message::message_id, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:525
  Message::message_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:525
  Message::message_size, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:525
  AttributeValueForObject::attribute_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:570
  AttributeValueForObject::attribute_value, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:570
  ValDescription::a, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:451
  ValDescription::b, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:451
  AttributeDefault::attribute_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:563
  AttributeDefault::attribute_value, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:563
  Signal::signal_size, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:294
  SignalType::signal_type_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:392
  SignalGroups::signal_group_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:615
  EnvironmentVariable::env_var_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:537
  EnvironmentVariable::env_var_type, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:537
  ValueTable::value_table_name, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:475
  ValueTable::value_descriptions, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:475

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_missing.ron

Failed in:
  struct can_dbc::DBC, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:642

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field signal_size of struct Signal, previously in file /tmp/.tmpBTmaKo/can-dbc/src/lib.rs:300
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [7.0.0](v6.0.0...v7.0.0) -
2025-10-18

### Added

- [**breaking**] support cp1252, rm `from_slice`, improve README
examples ([#44](#44))

### Other

- [**breaking**] major struct and enum naming refactoring
([#45](#45))
- update README with usage examples and license information
([#46](#46))
- [**breaking**] rename `DBC`→`Dbc` and feature `with-serde`→`serde`
([#42](#42))
- move tests to the end
([#43](#43))
- relicense as `MIT OR Apache-2.0`
([#38](#38))
- auto-release and `cargo deny`
([#39](#39))
- move test files to submodule, default with serde feature
([#40](#40))
- upload coverage reports
- update README to oxibus org
([#37](#37))
- allow space after message ID
([#25](#25))
- upgrade to nom 8 ([#36](#36))
- add automatic validation with precommit on CI side
([#30](#30))
- *(ci)* modernize CI ([#33](#33))
- cleanup a few clippy lints
- consolidate docs with readme
- use `insta` to test all parsing results
- use `clap-derive` in example
- run `cargo fmt`
- bump dependencies and minor cleanup
- Simplify tests, test for other escaped characters
- Remove obsolete is_quote function
- Add support for escaped strings within comments
- Also derive the Deserialize trait when serde is enabled
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants