Skip to content
This repository has been archived by the owner on Feb 18, 2022. It is now read-only.

INTEGER with SIZE checks in the Grammar #28

Open
pjanck opened this issue Dec 15, 2021 · 5 comments · May be fixed by #29
Open

INTEGER with SIZE checks in the Grammar #28

pjanck opened this issue Dec 15, 2021 · 5 comments · May be fixed by #29
Assignees

Comments

@pjanck
Copy link
Contributor

pjanck commented Dec 15, 2021

State

Documentation, chapter 4, grammar:

value : logical_literal | real_literal | regular_expression | string_literal;

Documentation, Table 19, Description of metric values:

SIZE: Indicates the size of a collection or STRING (value is an INTEGER).

Question

value cannot be type-safely an integer, although there is an int defined in the grammar. Thoughts?

Proposal

Expand value to include int_literal. I can prepare a PR if wished.

@MatthiasWeise
Copy link
Contributor

Having an int_literal might be a good option, although we could also argue that int is included in real_literal. In order to have more type safety we would have to bind the metrics SIZE to an int_literal. Can you make a proposal?

@SergejMuhic
Copy link

I would vote for int_literal instead of real_literal. It also rounds up the basic types nicely (apart from char, date and byte which are a bit debatable for checking purposes? 😄 ).

@MatthiasWeise
Copy link
Contributor

Yes for SIZE metrics, but we still need real_literal for VALUE metrics.

@SergejMuhic
Copy link

Misunderstanding. I have meant that instead of using real_literal also for an integer, we introduce int_literal.

@pjanck
Copy link
Contributor Author

pjanck commented Jan 24, 2022

we introduce int_literal
Can you make a proposal?

Yes, I'll prepare a PR.

In order to have more type safety we would have to bind the metrics SIZE to an int_literal.

This isn't that easy with the generality of the grammar. Consider that real_literal also doesn't make sense with TYPE checks - and that is not explicitly forbidden in the grammar. I believe that - as it currently stands - a note in the documentation suffices.

@pjanck pjanck linked a pull request Jan 24, 2022 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants