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

Duplicated issues #467

Closed
cgalvarez opened this issue May 9, 2017 · 2 comments · Fixed by #1026
Closed

Duplicated issues #467

cgalvarez opened this issue May 9, 2017 · 2 comments · Fixed by #1026
Assignees
Labels
Milestone

Comments

@cgalvarez
Copy link

I found duplicated issues, specifically for the rule CamelCaseParameterName (don't know if it happens with other rules), due to non-specific beginline and endline attributes. Maybe it's related to #223, but the errors reported in that issue are different.

Reproducible steps:

  1. Create a PHP file test.php with these contents:
<?php
function cge_theme_enqueue_styles() {
	$parent_style = 'twentyfifteen-style';
	wp_enqueue_style( $parent_style, get_template_directory_uri() . '/style.css' );
	wp_enqueue_style(
		'cge-site-style',
		get_stylesheet_directory_uri() . '/style.css',
		array( $parent_style ),
		wp_get_theme()->get( 'Version' )
	);
}
  1. Execute phpmd test.php xml controversial. You get the following output:
<?xml version="1.0" encoding="UTF-8" ?>
<pmd version="@project.version@" timestamp="2017-05-09T11:36:35+00:00">
  <file name="/opt/cgalvarez/wp-theme-cge-site/test.php">
    <violation beginline="2" endline="11" rule="CamelCaseVariableName" ruleset="Controversial Rules" package="+global" externalInfoUrl="#" function="cge_theme_enqueue_styles" priority="1">
      The variable $parent_style is not named in camelCase.
    </violation>
    <violation beginline="2" endline="11" rule="CamelCaseVariableName" ruleset="Controversial Rules" package="+global" externalInfoUrl="#" function="cge_theme_enqueue_styles" priority="1">
      The variable $parent_style is not named in camelCase.
    </violation>
    <violation beginline="2" endline="11" rule="CamelCaseVariableName" ruleset="Controversial Rules" package="+global" externalInfoUrl="#" function="cge_theme_enqueue_styles" priority="1">
      The variable $parent_style is not named in camelCase.
    </violation>
  </file>
</pmd>

As you can see, the variable $parent_style is used 3 times, at lines 3, 4 and 8, so I think the beginline and endline attributes should not be set to the function scope where the variable is used, but to the real line in which the variable is used.

My environment:

  • PHPMD 2.6.0
  • PHP 7.1.4
  • OS: Ubuntu Xenial 16.04
@ravage84 ravage84 added the Bug label May 9, 2017
@ravage84 ravage84 added this to the 2.6.1 milestone May 9, 2017
@ravage84
Copy link
Member

ravage84 commented May 9, 2017

Either it should show each line separately with the correct beginline and endline set or it should only be displayed once for the whole function.

@pointlessone
Copy link

It seems the issue is reported on the wrong node. The whole function node instead of just the variable node.

@kylekatarnls kylekatarnls modified the milestones: 2.7.1, 2.8.1 Oct 19, 2019
@kylekatarnls kylekatarnls modified the milestones: 2.8.1, 2.8.2 Dec 27, 2019
@kylekatarnls kylekatarnls modified the milestones: 2.8.2, 2.8.3 Feb 25, 2020
@kylekatarnls kylekatarnls modified the milestones: 2.8.3, 2.10.0 May 3, 2020
@kylekatarnls kylekatarnls modified the milestones: 2.10.0, 2.11.0 Apr 11, 2021
@kylekatarnls kylekatarnls modified the milestones: 2.11.0, 2.12.0 Nov 27, 2021
@kylekatarnls kylekatarnls modified the milestones: 2.12.0, 2.13.0 Jun 19, 2022
@kylekatarnls kylekatarnls modified the milestones: 2.13.0, 2.14.0 Sep 10, 2022
@kylekatarnls kylekatarnls self-assigned this Aug 21, 2023
kylekatarnls added a commit that referenced this issue Aug 21, 2023
And use line of first occurrence instead of line of parent function

Add test for uncovered exceptions

Fix #467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants