README: "AST doesn't know about spaces"

is this still pertinent?

Line 152 in 3e8c192

    
           * Rector uses [nikic/php-parser](https://github.com/nikic/PHP-Parser/), built on technology called an *abstract syntax tree* (AST). An AST doesn't know about spaces and when written to a file it produces poorly formatted code in both PHP and docblock annotations.

according to this issue, most issues with spacing were long since resolved?

in my preliminary testing with Rector, I have not seen it mangle any spaces - and that's in a codebase with no code standard 🙂

I understand there may still be some issues with spacing, but I almost skipped Rector (and would have done all of my rename operations by hand in PHP Storm instead) because of this warning - and clearly, it goes have some level of awareness of spacing, or this project would most likely have been totally mangled. 😅

(glad I chose Rector, as this will save me a lot of time! thank you for this incredible tool!)

Hi, thanks for the question. You're right 🙂 , it can be serious warning or not relevant at all, it depends on your codebase.

We've had many opposite reactions, where people wanted all the spaced to remain the same, but technically it was not possible, so we added this warning to expect the edge-case issues (depending on your code base) and be prepated.

@mindplay-dk

@mindplay-dk when no change on the target file, there is tweak on rector when comparing output from php-parser printer vs original file content:

rector/src/Application/FileProcessor.php

Lines 161 to 181 in 3e8c192

    
                   /** 
        
                    * When no diff applied, the PostRector may still change the content, that's why printing still needed 
        
                    * On printing, the space may be wiped, these below check compare with original file content used to verify 
        
                    * that no change actually needed 
        
                    */ 
        
                   if (!$file->getFileDiff() instanceof FileDiff && \current($file->getNewStmts()) instanceof FileWithoutNamespace) { 
        
                       /** 
        
                        * Handle new line or space before <?php or InlineHTML node wiped on print format preserving 
        
                        * On very first content level 
        
                        */ 
        
                       $ltrimOriginalFileContent = \ltrim($file->getOriginalFileContent()); 
        
                       if ($ltrimOriginalFileContent === $newContent) { 
        
                           return; 
        
                       } 
        
                       // handle space before <?php 
        
                       $ltrimNewContent = Strings::replace($newContent, self::OPEN_TAG_SPACED_REGEX, '<?php'); 
        
                       $ltrimOriginalFileContent = Strings::replace($ltrimOriginalFileContent, self::OPEN_TAG_SPACED_REGEX, '<?php'); 
        
                       if ($ltrimOriginalFileContent === $ltrimNewContent) { 
        
                           return; 
        
                       } 
        
                   }

with php-parser v5, the printed content no longer ltrim-ed, which we will revisit later once phpstan go to v2 which will require php-parser v5

Navigation Menu

Repository navigation

README: "AST doesn't know about spaces" #8537

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	/**
	* When no diff applied, the PostRector may still change the content, that's why printing still needed
	* On printing, the space may be wiped, these below check compare with original file content used to verify
	* that no change actually needed
	*/
	if (!$file->getFileDiff() instanceof FileDiff && \current($file->getNewStmts()) instanceof FileWithoutNamespace) {
	/**
	* Handle new line or space before <?php or InlineHTML node wiped on print format preserving
	* On very first content level
	*/
	$ltrimOriginalFileContent = \ltrim($file->getOriginalFileContent());
	if ($ltrimOriginalFileContent === $newContent) {
	return;
	}
	// handle space before <?php
	$ltrimNewContent = Strings::replace($newContent, self::OPEN_TAG_SPACED_REGEX, '<?php');
	$ltrimOriginalFileContent = Strings::replace($ltrimOriginalFileContent, self::OPEN_TAG_SPACED_REGEX, '<?php');
	if ($ltrimOriginalFileContent === $ltrimNewContent) {
	return;
	}
	}

README: "AST doesn't know about spaces" #8537

Description

Activity

TomasVotruba commented on Mar 11, 2024

samsonasik commented on Mar 11, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions