Hello Tom,

It is a nice idea to add this as an option. I think the most easy option is to add a command line option that strip the base directory from the filename.
The fullpath is set / fixed in the ASTCompilationUnit class from pdepend. (See: https://github.com/pdepend/pdepend/blob/master/src/main/php/PDepend/Source/AST/ASTCompilationUnit.php#L129 ) So it is more logic to trim the running path from the filepath in therenderer.

Feel free to create a PR for this.

Hello Tom,

It is a nice idea to add this as an option. I think the most easy option is to add a command line option that strip the base directory from the filename.
The fullpath is set / fixed in the ASTCompilationUnit class from pdepend. (See: https://github.com/pdepend/pdepend/blob/master/src/main/php/PDepend/Source/AST/ASTCompilationUnit.php#L129 ) So it is more logic to trim the running path from the filepath in de renderer.

Feel free to create a PR for this.

I think it’d be best if we introduce a file_name key alongside the original file. What do you think, @tvbeek?

Yes, good idea, fileName to respect the camelCase used in other keys. But this way, we avoid having a new option to document. :)

Yes, good idea, fileName to respect the camelCase used in other keys. But this way, we avoid having a new option to document. :)

Would you store this next to file or do you prefer it under the violations key? I’d personally store it next to file.

Next to file sounds good for me. 👍

But maybe is fileName not a correct one, in your example I should suspect that fileName is Class.php. Maybe relativeFile or something like that.

What about baseName referring https://www.php.net/manual/en/function.basename.php

I'd opt for baseName then.

Perfect.

/**
 * Returns the base name where this rule violation was detected.
 *
 * @return string
 */
public function getBaseName()
{
    var_dump($this->getFileName());
    var_dump(basename($this->getFileName()));
    return $this->getFileName();
}

Currently outputs:

/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:153:
string(36) "/var/www/html/app/Console/Kernel.php"
/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:154:
string(10) "Kernel.php"

It strips out too much information, at the moment. I'll try and find the correct PHP function to use.

Oups, my mistake, indeed baseName was not what we're looking for.

Psalm does it as follows: https://github.com/vimeo/psalm/blob/a154191922d14dcdde8ef348520a88d735d82304/src/Psalm/Config.php#L1162 but I'm sure there must be an easy/easier way?

It's about relative path (implicitly relative to current directory getcwd which is where you run the phpmd command.

And the Psalm solution will still return an absolute path for paths outside the baseDir (but I guess this case can't happen).

Relative path from 2 paths can be calculated this way:

$cwd = explode(DIRECTORY_SEPARATOR, getcwd());
$path = explode(DIRECTORY_SEPARATOR, realpath('some-directory/some-file.php'));

foreach ($cwd as $index => $directory) {
    if ($directory !== $path[$index]) {
        break;
    }

    unset($path[$index]);
    unset($cwd[$index]);
}

echo str_repeat('../', count($cwd)).implode('/', $path);

This will be correct even if you run phpmd ../other-directory text naming.xml

In our case $this->fileName is already a realpath and we may check if it's better to standardize output with / separator for relative path or use the DIRECTORY_SEPARATOR to respect the OS settings. I vote for / so this relativePath will be the same no matter in which OS it's ran.

I’ll give this a whirl in a bit. Cheers!

It's about relative path (implicitly relative to current directory getcwd which is where you run the phpmd command.

And the Psalm solution will still return an absolute path for paths outside the baseDir (but I guess this case can't happen).

Relative path from 2 paths can be calculated this way:

$cwd = explode(DIRECTORY_SEPARATOR, getcwd());
$path = explode(DIRECTORY_SEPARATOR, realpath('some-directory/some-file.php'));

foreach ($cwd as $index => $directory) {
    if ($directory !== $path[$index]) {
        break;
    }

    unset($path[$index]);
    unset($cwd[$index]);
}

echo str_repeat('../', count($cwd)).implode('/', $path);

This will be correct even if you run phpmd ../other-directory text naming.xml

This did the trick in my initial test!

/**
     * Returns the base name where this rule violation was detected.
     *
     * @return string
     */
    public function getBaseName()
    {
        $cwd = explode(DIRECTORY_SEPARATOR, getcwd());
        $path = explode(DIRECTORY_SEPARATOR, realpath($this->getFileName()));

        foreach ($cwd as $index => $directory) {
            if ($directory !== $path[$index]) {
                break;
            }

            unset($path[$index]);
            unset($cwd[$index]);
        }

        var_dump(str_repeat('../', count($cwd)).implode('/', $path));

        return $this->getFileName();
    }

/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:165:
string(22) "app/Console/Kernel.php"
/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:165:
string(33) "app/DisallowTraitsInNamespace.php"
/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:165:
string(48) "app/Http/Controllers/Auth/RegisterController.php"
/var/www/html/vendor/phpmd/phpmd/src/main/php/PHPMD/RuleViolation.php:165:
string(12) "app/User.php"

The new output is as follows:

{
    "version": "@project.version@",
    "package": "phpmd",
    "timestamp": "2019-12-06T10:52:04+00:00",
    "files": [
        {
            "file": "\/var\/www\/html\/app\/Console\/Kernel.php",
            "baseName": "app\/Console\/Kernel.php",
            "violations": [
                {
                    "beginLine": 25,
                    "endLine": 25,
                    "package": null,
                    "function": null,
                    "class": null,
                    "method": null,
                    "description": "Avoid unused parameters such as \u0027$schedule\u0027.",
                    "rule": "UnusedFormalParameter",
                    "ruleSet": "Unused Code Rules",
                    "externalInfoUrl": "https:\/\/phpmd.org\/rules\/unusedcode.html#unusedformalparameter",
                    "priority": 3
                }
            ]
        }
    ]
}

Thanks @ttomdewit! Would you be OK to create a pull-request for that and ideally add a unit test that cover this new feature? So you'll get your name on this contribution.

Thanks @ttomdewit! Would you be OK to create a pull-request for that and ideally add a unit test that cover this new feature? So you'll get your name on this contribution.

I would very much like that! I'll try and get that sorted today!

I started working on a PR: #706

Thanks! In this PR we can focus on JSON renderer but I think some other renderers (like the XML one) may also benefit of this adding.

Thanks! In this PR we can focus on JSON renderer but I think some other renderers (like the XML one) may also benefit of this adding.

I agree. Maybe we can move the new method up so all renderers benefit from it?

See #706 (comment) for details

Yes, what the actual goal here is the "relative path", nothing else. Thus, we should name it like that.

I'm going to edit the title of this issue, too, to reflect the new goal.

In our case $this->fileName is already a realpath and we may check if it's better to standardize output with / separator for relative path or use the DIRECTORY_SEPARATOR to respect the OS settings. I vote for / so this relativePath will be the same no matter in which OS it's ran.

I guess this is out of scope of this issue/PR. And we cannot change existing report field data becaus of backwards compatibility.