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

This modules LESS Analyzer class cannot handle LESS files which use @import url() on css files #73

Open
furan917 opened this issue Oct 6, 2023 · 2 comments

Comments

@furan917
Copy link

furan917 commented Oct 6, 2023

When using this tool it was discovered that if you have a module that uses an @import url() on a css file the analyzer would break with the following error:

PHP Fatal error:  Uncaught Error: Object of class Less_Tree_Quoted could not be converted to string in magento-semver/src/Analyzer/Less/Analyzer.php:131

Take for example if your branch(es) included AdminAdobeIms which does this here.

The reason behind this is because when parsing over the modules content it converts the import to a Link_Tree_Import which is comprised of a Link_Tree_Uri as the path and a Link_Tree_Quoted as the value.

This means that the check done here is not sufficient enough to handle it.

You could handle it by adding another guard check near the top of the foreach (or expanding the current one)

                    if ($node instanceof Less_Tree_Import) {
                        continue;
                    }

or specifically handle the nodeKey set with something like

                        if ($node->path->value instanceof \Less_Tree_Quoted || property_exists($node->path->value, 'value')) {
                            $nodeKey = $node->type . ' with value: \'' . $node->path->value->value . '\'';
                        } else {
                            $nodeKey = $node->type . ' with value: \'' . $node->path->value . '\'';
                        }
@m2-assistant
Copy link

m2-assistant bot commented Oct 6, 2023

Hi @furan917. Thank you for your report.
To speed up processing of this issue, make sure that you provided sufficient information.
Add a comment to assign the issue: @magento I am working on this


Join Magento Community Engineering Slack and ask your questions in #github channel.

@furan917 furan917 changed the title This modules Analyzer class cannot handle LESS files which use @import url() on css files This modules LESS Analyzer class cannot handle LESS files which use @import url() on css files Oct 6, 2023
furan917 added a commit to furan917/magento-semver that referenced this issue Oct 6, 2023
furan917 added a commit to furan917/magento-semver that referenced this issue Oct 6, 2023
Related to magento#73 - option 2, if you wish to see a more indepth writeup

The Less Analyzer class cannot handle Less files that import css as the object they convert into mean the $node->path->value is a Link_Tree_Quoted object and not a string castable value, so we add an additional check for if we are a CSS quoted file or if value is an object which contains another value and use that instead (In the off chance similar future issues may arise with new objects)
@furan917
Copy link
Author

furan917 commented Oct 6, 2023

For the sake of ease, I have created a PR for both options so you can see the direct changes I proposed to help better understand my point.

Option 1, Guard clause, this might be too aggressive of an option

Option 2, Expanded conditional

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant