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

Empty FUNCTION statement detected should be allowed for around plugins #95

Closed
markshust opened this issue May 11, 2019 · 1 comment
Closed
Assignees
Labels
accepted New rule is accepted enhancement Improvements to existing rules false positive Rule causes false positive findings Progress: good first issue Issues is easy to get started with

Comments

@markshust
Copy link

Description

A WARNING is thrown for sniff "Empty FUNCTION statement detected"

This warning should not be through if the empty function is an around plugin. A common use-case for around plugins is to prevent execution of the original calling method. For example:

    public function aroundPrepareFavicon(Renderer $subject, callable $proceed)
    {
        // Do not call $proceed() to remove favicon code
    }

Expected behavior

I would expect this sniff to still run and throw a WARNING in all circumstances other than an around plugin. The sniff should not throw a WARNING for an around plugin.

A consideration may be to mandate the use of a comment for an around plugin with an empty function statement. This would ensure that the empty around plugin not calling the callable $proceed is desired. See above for an example comment which explains why this around plugin is empty.

Benefits

Empty around plugins are not only valid but this is a common use-case. The sniff should not detect this as an error. A comment within an empty around plugin would also most likely be desired and document to the user why this around function is empty.

@markshust markshust added the enhancement Improvements to existing rules label May 11, 2019
@lenaorobei lenaorobei added false positive Rule causes false positive findings need to discuss Rule requires discussion accepted New rule is accepted labels May 13, 2019
@lenaorobei lenaorobei added this to Backlog in Community Dashboard via automation May 17, 2019
@lenaorobei lenaorobei moved this from Backlog to Up for grabs in Community Dashboard May 17, 2019
@lenaorobei lenaorobei added Progress: good first issue Issues is easy to get started with and removed need to discuss Rule requires discussion labels Jun 5, 2019
@okorshenko okorshenko added this to Ready for Grooming in Backlog Jun 12, 2019
@m2-community-project m2-community-project bot moved this from Ready for Grooming to Good First Issue in Backlog Jun 12, 2019
@larsroettig larsroettig self-assigned this Aug 7, 2019
@m2-community-project m2-community-project bot moved this from Good First Issue to Dev In Progress in Backlog Aug 7, 2019
lenaorobei added a commit that referenced this issue Aug 9, 2019
#95 : Empty FUNCTION statement detected should be allowed for around plugins
@lenaorobei
Copy link
Contributor

#126

@m2-community-project m2-community-project bot moved this from Dev In Progress to Done in Backlog Aug 13, 2019
magento-devops-reposync-svc pushed a commit that referenced this issue Oct 6, 2021
…oding-standard-309

[Imported] AC-683: Create phpcs static check for TableTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted New rule is accepted enhancement Improvements to existing rules false positive Rule causes false positive findings Progress: good first issue Issues is easy to get started with
Projects
Backlog
  
Done
Community Dashboard
  
Up for grabs
Development

No branches or pull requests

4 participants