mozart icon indicating copy to clipboard operation
mozart copied to clipboard

Namespace added to `as` keyword

Open ziodave opened this issue 4 years ago • 8 comments

The original file ./vendor/pear/pear_exception/PEAR/Exception.php:195 has the following line:

foreach (self::$_observers as $func) {

The replaced file ./classes/dependencies/pear/pear_exception/PEAR/Exception.php:195 has the following line:

foreach (self::$_observers Project_as $func) {

It looks as if as is replaced with Project_as.

This is the composer.json:

{
  "autoload": {
    "classmap": [
      "classes/dependencies/"
    ]
  },
  "require-dev": {
    "coenjacobs/mozart": "dev-master",
    "cweagans/composer-patches": "^1.6"
  },
  "extra": {
    "patches": {
      "coenjacobs/mozart": {
        "Replace parent packages over full tree of dependencies.": "https://github.com/coenjacobs/mozart/pull/82.patch",
        "exclude_files": "https://github.com/coenjacobs/mozart/pull/83.patch",
        "Improve ClassmapReplacer prefixing": "https://github.com/coenjacobs/mozart/pull/85.patch"
      }
    },
    "mozart": {
      "dep_namespace": "Project\\Dependencies\\",
      "dep_directory": "/src/Dependencies/",
      "classmap_directory": "/classes/dependencies/",
      "classmap_prefix": "Project_",
      "delete_vendor_directories": false,
      "exclude_files_from_copy": [
        "/\\.*composer\\.json/"
      ],
      "override_autoload": {
        "pear/net_url2": {
          "classmap": [
            "Net/"
          ]
        },
        "pear/mail_mime": {
          "classmap": [
            "Mail/"
          ]
        },
        "pear/console_getopt": {
          "classmap": [
            "Console/"
          ]
        },
        "pear/pear-core-minimal": {
          "classmap": [
            "src/"
          ]
        }
      }
    }
  },
  "require": {
    "microsoft/windowsazure": "*"
  },
  "repositories": [
    {
      "type": "git",
      "url": "https://github.com/coenjacobs/mozart.git"
    }
  ]
}

ziodave avatar Nov 23 '20 10:11 ziodave

I am unable to understand where that happens. The regex (?:[abstract]*class |interface )([a-zA-Z0-9_\x7f-\xff]+)\s?(?:\n*|{| extends| implements) doesn't seem to be the culprit:

image

ziodave avatar Nov 23 '20 11:11 ziodave

Actually the issue appears to be there, after adding some logging to ./vendor/coenjacobs/mozart/src/Replace:15/ClassmapReplacer.php:

        return preg_replace_callback(
            '/(?:[abstract]*class |interface )([a-zA-Z0-9_\x7f-\xff]+)\s?(?:\n*|{| extends| implements)/',
            function ($matches) {
                $replace = $this->classmap_prefix . $matches[1];
                error_log( 'Replacing ' . $matches[1] . ' with ' . $replace . ' [' . $matches[0] . ']');
                $this->saveReplacedClass($matches[1], $replace);
                return str_replace($matches[1], $replace, $matches[0]);
            },
            $contents
        );

I can see, but it's as if the line is changed before the regex is applied, since the actual line is foreach (self::$_observers as $func), not class as:

Replacing "as" with "Project_as" [class as ]

ziodave avatar Nov 23 '20 15:11 ziodave

Ok, I can see what happens:

image

In comments there's a class as which is interpreted as a class name and later replaced.

ziodave avatar Nov 23 '20 15:11 ziodave

I revised the regular expression as follows:

/^(?!\s*[*\/]).*(?:(?:abstract)*class |interface )([a-zA-Z0-9_\x7f-\xff]+)\s?(?:\n*|{| extends| implements)/m

This new part ^(?!\s*[*\/]).* (along with the multiline modifier) will ensure that the line doesn't start with * or with / which signal a comment line.

I also changed [abstract] to (?:abstract) since [abstract] would match any sequence of those characters (e.g. abb): image

It fixes the issue for me. Let me know if you'd like me to do a PR.

ziodave avatar Nov 23 '20 15:11 ziodave

Another edgy case:

image

ziodave avatar Nov 23 '20 15:11 ziodave

This seems to work for me:

/^(?!\s*[*\/])\s*(?:(?:abstract\s+)?class |interface )([a-zA-Z0-9_\x7f-\xff]+)\s?(?:\n*|{| extends| implements)/m

I changed (?!\s*[*\/]).* to (?!\s*[*\/])\s*, i.e. only white space characters allowed until one of abstract, class or interface is found.

ziodave avatar Nov 23 '20 16:11 ziodave

I think

$myvar = 123; class Pear { };

is valid too. So the the beginning ^ should maybe be ^(?:.*;)? I tested a little but I really need to read some documentation to check I'm using the right syntax.

Definitely do a PR. The tests for regex are pretty easy to write. If you don't, I'll take your work and comments here and type it up in the next couple of weeks.

BrianHenryIE avatar Nov 23 '20 20:11 BrianHenryIE

getting same thing with to

image

	"extra": {
		"mozart": {
			"dep_namespace": "MyVendor\\Dependencies\\",
			"dep_directory": "/Dependencies/",
			"classmap_directory": "/Dependencies/autoload/",
			"classmap_prefix": "EECD_",
			"packages": [
				"dompdf/dompdf",
				"endyjasmi/cuid",
				"tijsverkoyen/css-to-inline-styles",
				"jaybizzle/crawler-detect",
				"wp-graphql/wp-graphql"
			],
			"delete_vendor_directories": true
		}
	},
	"scripts": {
		"post-install-cmd": [
			"\"vendor/bin/mozart\" compose",
			"composer dump-autoload"
		],
		"post-update-cmd": [
			"\"vendor/bin/mozart\" compose",
			"composer dump-autoload"
		]
	}

tn3rb avatar Apr 26 '23 20:04 tn3rb