Added a default value for VersionSpecifier and some documentation

[teofr]

Originally I wanted to stop wrapping VersionSpecifier in an Option, and parse it as Always if it's not present, but that brings some issues with the span information, and we'd need to differentiate between an Always with span (explicit) and one without (implicit).

However, I think having the default value representable is good to simplify some algorithms, in particular, the information of what a missing field on the language definition means is now explicit.

I also added some documentation here and there.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7b1d5fc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ggiraldez]

Left some questions and suggestions.

[ggiraldez]

I don't have any more particular observations, but a more general question: do we need to differentiate between absent version specifier and Always? I don't see a reason to keep both possible states, and in that case we should change all other instances (eg. in StructItem, EnumItem, etc) of VersionSpecifier to remove the Option<>.

[teofr]

(...) do we need to differentiate between absent version specifier and Always? (...)
@ggiraldez

The main reason is for the code generation engine, we expect most items of a parsed AST (of a language definition) to always have a span, most of the logic to derive spanned types works that way. So a SpannedVersionSpecifier has a Span, always. An Option<SpannedVersionSpecifier> may not have a span (if it's absent).

As far as I could see we treat Option<...> as special cases sometimes, I think is possible to have similar logic for Default structs (like the new VersionSpecifier) but you'd need to have some way of either differentiating between an explicit Always and an implicit one, or guarantees that no Always will need a span (no error messages).

What this change does allow is that further down the stream, for example when building a lexer, we can simply use .unwrap_or_default().

I agree it'd be a bigger win in ergonomics if we could get rid of the Option entirely, but it needs more work.

[ggiraldez]

I agree it'd be a bigger win in ergonomics if we could get rid of the Option entirely, but it needs more work.

I see. I wasn't considering the model generation layer (partly because I haven't explored that part of the code sufficiently) and was thinking in terms of the model as a result, but they are tightly coupled. Thanks for the explanation!

[OmarTawfik]

I suggest removing crates/solidity-v2/inputs/language/bindings/rules.msgb as well.