Skip to content

bug(#208): incorrect-package accepts fqn#211

Merged
yegor256 merged 5 commits intoobjectionary:masterfrom
h1alexbel:208
Jan 10, 2025
Merged

bug(#208): incorrect-package accepts fqn#211
yegor256 merged 5 commits intoobjectionary:masterfrom
h1alexbel:208

Conversation

@h1alexbel
Copy link
Member

@h1alexbel h1alexbel commented Jan 9, 2025

In this pull I've updated incorrect-package lint to support FQN names.
see #208

@h1alexbel
Copy link
Member Author

waiting for #213

@h1alexbel
Copy link
Member Author

@volodya-lombrozo could you check this one, please?

@yegor256
Copy link
Member

@h1alexbel this is full regexp for FQN in EO: https://github.com/objectionary/eo/blob/master/eo-parser/src/main/resources/XMIR.xsd#L99 Maybe it's better to fix our matcher once and for all, not only for the $ sign. For example, this is a legit code:

+package a.b-привет.c-大家好

Will our current lint pass it?

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Despite having lowercase letters and _, $ signs, Java package might contain uppercase letters:

The specification allows it [uppercase characters] just fine. It's only a convention to use all-lower-case.

From: https://stackoverflow.com/a/12534341/10423604

@h1alexbel h1alexbel changed the title bug(#208): incorrect-package accepts name with $ bug(#208): incorrect-package accepts fqn Jan 10, 2025
@h1alexbel
Copy link
Member Author

@volodya-lombrozo updated. Take a look, please

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Looks good to me, thank you!

@h1alexbel
Copy link
Member Author

@yegor256 take a look, please

@yegor256 yegor256 merged commit dd3969f into objectionary:master Jan 10, 2025
12 checks passed
@yegor256
Copy link
Member

@h1alexbel thanks!

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.

3 participants