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.

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

NotContainDocBlockLike matches not related classes #363

Open
Maxell92 opened this issue Feb 24, 2023 · 7 comments
Open

NotContainDocBlockLike matches not related classes #363

Maxell92 opened this issue Feb 24, 2023 · 7 comments

Comments

@Maxell92
Copy link

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

We want to be sure, no one throws a general Exception in functions:

	$rules[] = Rule::allClasses()
		->that(new ResideInOneOfTheseNamespaces('App'))
		->should(new NotContainDocBlockLike('@throws Exception'))
		->because('we don\'t want simple exceptions - use custom instead');

The rule also matches the case below, which is not useful:

	/**
	 * @throws ExceptionInterface
	 */
	public function doStuff(): void
	{
		doSomething();
	}

The violation:

Xxx has 1 violations
  should not have a doc block that contains @throws Exception because we don't want simple exceptions - use custom instead

Is there any possibility to use a regex?

@micheleorselli
Copy link
Contributor

Hi @Maxell92,
thank you for reporting this 🙇
We could probably make NotContainDocBlockLike accepting wildcards as for the rules that match class name or namespaces. What do you think @fain182 @AlessandroMinoccheri?

@AlessandroMinoccheri
Copy link
Member

Yes, it could be useful to accept wildcards!

@fain182
Copy link
Collaborator

fain182 commented Feb 27, 2023

@micheleorselli it makes sense...
So you would break backward compatiblity and transform NotContainDocBlockLike in an exact match when there are no wildcard?

@micheleorselli
Copy link
Contributor

Or we could add a new NotContainDocBlock and deprecate the usage of NotContainDocBlockLike

@AlessandroMinoccheri
Copy link
Member

I prefer to add a new NotContainDocBlock rule.

@LuigiCardamone
Copy link
Contributor

Just brainstorming. Maybe it can be useful to add a specif rule for Exceptions. The rule ->should(new NotContainDocBlockLike('@throws Exception')) does not work well if the same Exception is used with a relative namespace (E.g. @throws Exception) or with the complete namespace (E.g. @throws \Exception)

@micheleorselli
Copy link
Contributor

micheleorselli commented Mar 2, 2023

I think I get what you mean... we should probably look into @throw tags during the analysis and treat them as dependencies

That said, a NotContainDocBlock could still be useful

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

No branches or pull requests

5 participants