Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

False Negative mandatory-package Check #234

Closed
volodya-lombrozo opened this issue Jan 13, 2025 · 10 comments
Closed

False Negative mandatory-package Check #234

volodya-lombrozo opened this issue Jan 13, 2025 · 10 comments

Comments

@volodya-lombrozo
Copy link
Member

I have an XMIR file with the following meta:

<metas>
  <meta>
    <head>version</head>
    <tail>1.2.3</tail>
  </meta>
</metas>

I added it to handle mandatory-version check and immediately have got the following error:

XMIR is incorrect: [[mandatory-package WARNING]:0 The +package meta is mandatory, but is absent]

It used to work well because if I had empty package I avoided adding metas at all. Now I have non-empty metas and empty package. objectionary/lints complaints about it.

Please remove this check or fix it. Otherwise, the checks conflicts with each other.
@volodya-lombrozo
Copy link
Member Author

@yegor256 Could you have a look, please?

@volodya-lombrozo
Copy link
Member Author

Blocks objectionary/jeo-maven-plugin#962

@yegor256
Copy link
Member

@volodya-lombrozo I believe, mandatory-package complains about the absence of +package meta in any case, either +version is there or not. This is by design. Or I misunderstand the problem?

@volodya-lombrozo
Copy link
Member Author

volodya-lombrozo commented Jan 13, 2025 •
edited
Loading

@volodya-lombrozo I believe, mandatory-package complains about the absence of +package meta in any case, either +version is there or not. This is by design. Or I misunderstand the problem?

@yegor256 It's legal to have empty package:

@volodya-lombrozo when package is not declared, don't put package meta in the XMIR. Very often we don't have packages, it's legal.

They are you words, actually: objectionary/jeo-maven-plugin#912 (comment)

Thus, I don't have package for some objects, but objectionary/lints complaints about it.

@yegor256
Copy link
Member

yegor256 commented Jan 13, 2025 •
edited
Loading

@volodya-lombrozo it's legal, but it's better to have it (that's why it's a warning, not an error). Maybe you can use some global package for all objects in JEO, something like java. Thus, if there are no packages, all objects are java.foo (for example). If it's org.springframework.Foo, you make it java.org.springframework.Foo. Another option is jeo.

@volodya-lombrozo
Copy link
Member Author

@yegor256 I believe it's better just to remove this check. We do have empty packages. It's perfectly legal. The absence of the package entry doesn't influence anything. It's not an anti-pattern and doesn't break anything. I don't see any reason for leaving the check.

Moreover, please, pay attention, I don't have package entry at all:

<metas>
  <meta>
    <head>version</head>
    <tail>1.2.3</tail>
  </meta>
</metas>

If I would have

<metas>
  <meta>
    <head>version</head>
    <tail>1.2.3</tail>
  </meta>
  <meta>
    <head>package</head>
    <tail></tail>
  </meta>
</metas>

I would hardly agree (no) that it might be a warning.

@yegor256
Copy link
Member

@volodya-lombrozo remember, the lints exist not only for XMIR checking, but mostly for EO programmers. We want them to be as disciplined as possible, that's why lints are as strict as we can make them. Having an object without a package means having it in the global scope of visibility (the entire "universe" should see it and use it). This is not a good idea.

@volodya-lombrozo
Copy link
Member Author

volodya-lombrozo commented Jan 13, 2025 •
edited
Loading

@yegor256 Maybe I can ignore this check somehow? I don't need this rule for XMIR checking.

@yegor256
Copy link
Member

@volodya-lombrozo sure, you can add this and the rule will be ignored:

<metas>
  <meta>
    <head>unlint</head>
    <tail>mandatory-package</tail>
  </meta>
</metas>

@volodya-lombrozo
Copy link
Member Author

@volodya-lombrozo sure, you can add this and the rule will be ignored:

<metas>
  <meta>
    <head>unlint</head>
    <tail>mandatory-package</tail>
  </meta>
</metas>

@yegor256 Since we can ignore this check, I believe we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yegor256 @volodya-lombrozo