Fix panic in tedge_config macro when - present in field name

[Bravo555]

Proposed changes

#[tedge_config(rename = "name-with-hyphens")] panicked because of hyphens in the rename field. This occured because a test that was being generated to test that field did not remove hyphens from the identifier name, e.g:

error: proc macro panicked
...
= help: message:
"example_value_can_be_deserialized_for_run_name-with-hyphens_example_0" is not a valid identifier

Was fixed by replacing hyphens with underscores in identifier name.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[Bravo555]

One thing I noticed is that the panic was not triggered with #[tedge_config(rename = "hyphen-separated-field")] and I also had to add example = hsf in the macro. Is this intended behaviour?

[jarhodes314]

Yes, the generated tests exist to verify the examples are valid, so there is no test if no example is provided, and if there was a second example two tests would be generated.

[codecov]

Codecov Report

Merging #2220 (cb5cd77) into main (8edc2d8) will increase coverage by 0.9%.
Report is 1 commits behind head on main.
The diff coverage is 100.0%.

Additional details and impacted files

Files Changed	Coverage Δ
crates/common/tedge_config_macros/impl/src/lib.rs	95.2% <100.0%> (+0.3%)	⬆️

... and 6 files with indirect coverage changes

[jarhodes314]

This looks good to me. Out of interest, what are you intending to use hyphens for? I'm just wondering as we use underscores elsewhere in tedge.toml, and using a different casing style may be confusing.

[github-actions]

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
261	0	5	261	100	51m28.017s

[reubenmiller]

This looks good to me. Out of interest, what are you intending to use hyphens for? I'm just wondering as we use underscores elsewhere in tedge.toml, and using a different casing style may be confusing.

Yes I would not advise to use hypens as this makes translating the configuration name to the environment variable equivalent more complicated as environment variables should only match the following regex (for compatibility): ^[A-Z][A-Z0-9_]*$

[Bravo555]

Out of interest, what are you intending to use hyphens for? I'm just wondering as we use underscores elsewhere in tedge.toml, and using a different casing style may be confusing.

I was just testing how the config options would look like with hyphens, and stumbled on this bug. I don't intend to use them for any configuration options.

[jarhodes314]

This looks good to me. Out of interest, what are you intending to use hyphens for? I'm just wondering as we use underscores elsewhere in tedge.toml, and using a different casing style may be confusing.

Yes I would not advise to use hypens as this makes translating the configuration name to the environment variable equivalent more complicated as environment variables should only match the following regex (for compatibility): ^[A-Z][A-Z0-9_]*$

Obviously this bug was never the right behaviour, but I'm wondering whether characters that can't be (widely) used in environment variable names should be explicitly rejected with an appropriate compiler error explaining why we don't support them, in case someone accidentally uses incorrect casing in the future.