Debug / stack trace / some kind of help :-)

[cxj]

Feature Request

It would be nice if errors generated within Transphporm code were more helpful. For example, I'm trying to figure out why I am getting this error message (and no other output):

vendor/level-2/transphporm/src/Property/Content.php on line 79

So far my debugging seems to indicate it's the result of the $value variable in the function attr() call containing an array of one element, which itself is an empty array. But how I caused this, I don't know. I suspect faulty $data passed from my TSS, but am not sure.

Some kind of Transphpom-provided debug facility or stack backtrace generation or something would be really nice to have.

[solleer]

I like that idea. Don't quite know how it would work. Will think about it though it might be a while.

[TRPB]

There is format: debug which does a var_dump e..g. div.debug {content: data(); format: debug} letting you see what data is attached to an element. A stack trace might be useful but I think we'd need to cut out all of the internal transphporm calls as it wouldn't help debugging.

[cxj]

I was thinking of some kind of static debug flag that could be set within Transphporm by client code. Then when Transphporm threw a PHP error, some kind of debug output could be generate showing which element caused the problem.

The most frequent problem I've run into is where the entire web page HTML will disappear, and either it will be blank white, or I'll have a one line error like the one in the issue above. Often the PHP error is something like Array to string conversion in <filename> on line <line> which is some kind of generic data error. Since I'm parsing at least two XML files and two TSS files for most of my web pages (often more, e.g.: header, menu, footer, main data, default layout), it's very hard to tell which of many potential elements is causing the trouble.

[cxj]

How about this suggestion, as an alternative or first step. When Transphporm gets a fatal error, output the XML and TSS token being handled at that moment.

Virtually every such fatal error I've gotten has been the result of something like TSS referring to data element it cannot find or is of the wrong type (e.g. an array when string was expected).

If the error message said something like:

Error while evaluating content: iteration(filename); in file mypage.tss

That would be super helpful.

I realize in some cases this won't be easy or feasible.

[cxj]

Most desirable would be a call my application code could make into the Transphporm classes to ask for the current TSS token being parsed at the time an error is caught. That way, I can tell PHP to always call my own custom error code whenever PHP generates and error condition, and then I can choose what to do about it. It would be asking too much to add that kind of choice and code into Transphporm.

I think this means some kind of parsing status object needs to be injected into the token handler classes, and a line or two of code added to update the status object where needed. That's probably in methods which parse or try to dereference (obtain values from) provided data (i.e. the $data in a call to Transphporm\Builder::output($data) method).

I'm trying to figure out how the parser works well enough to hack in a proof of concept hook of some sort, but haven't gained enough understanding yet.

[solleer]

I like that idea. Before I work on it though I need to get the Token class done and in order for my to feel good about merging that branch I want @TomBZombie to look at it because is has some issues.

[cxj]

What needs to happen for the Token class to be done?

Would you be willing to let me polish the documentation and style? (I'm finding it harder to figure out what's going on because of lack of PHPDoc-style documentation and unconventional code style in Transphporm.)

[solleer]

The class just feels sloppy. Its in the tokens-class branch. I think it might be breaking encapsulation and the way the to and from functions in it work bother me a bit because I have it always start after the current iteration in a loop.

[solleer]

Also what is unconventional about the code style?

[cxj]

Here's one example (some lines removed for brevity):

        if (is_file($this->tss)) {
            if (!$rules) return $this->cache->write($key, (new Parser\Sheet(file_get_contents($this->tss), $this->baseDir, $config->getCssToXpath(), $config->getValueParser()))->parse());
            else return $rules;
        }
        else return (new Parser\Sheet($this->tss, $this->baseDir, $config->getCssToXpath(), $config->getValueParser()))->parse();

[cxj]

I suppose that might look ok if you edit with a screen/line width long enough not wrap the calls to new Parser\Sheet at all, but that seems like an unlikely situation for most people.

[cxj]

Even absent code style, it'd be nice to have PHPDoc blocks for each method and class. :-)

[solleer]

I certainly agree that PHPDocs need to be added

[TRPB]

@cxj are you able to provide sample XML/TSS which causes a blank output?

There are two different errors we need to account for:

• XML errors (although DomDocument should in theory handle this for us)
• TSS Errors

[cxj]

I'll see if I can reproduce the blank output. I haven't seen that particular problem in a while though.

Regarding the two different errors -- yes. TSS errors seem to be far more frequent and harder to find. XML is easy to pre-check with XML validity tools. TSS mostly fails when it doesn't get the data values from the calling PHP code. It'd be nice to have a way to have a user-provided function handle those, with some kind of reference to the element being substituted, e.g. a hook or something. I don't expect Transphporm to do the right thing, but rather am just asking it give me a little information so I can properly handle and debug it myself. :-)

[solleer]

There could be a \Transphporm\Exception sent by Transphporm if it encounters an error and that could contain the original exception as well as the tss and xml

[cxj]

Could it include the specific TSS element which failed, versus say, the entire file? Just to be sure we are thinking the same thing. :-)

If so, that would be great!

[solleer]

Yes, the specific tss

[solleer]

I'm going to work on this

[solleer]

Look at the debug-exception branch. The main problem is getting the line of the error. @TomBZombie Passing the line might require some changes.

[cxj]

Will take a look. Thanks for the effort!

[cxj]

There's a missing catch for the try in Hook/PseudoMatcher.php at about line 37.

[cxj]

I'm having trouble causing an exception to be thrown. It used to be that if I simply did something like:

$xml = '<p>Place Holder</p>';
$tss = 'p { content: data(name); }';
$tpl = new Transphporm\Builder($xml, $tss);
echo $tpl->output('')->body;

It would throw an error in the error log about using an array as a string.

I've tried a number of variations without any errors. It's as though the code looking for values for content: data(...) clauses is now more fault tolerant, like maybe it's assuming the proper type empty value when no value or the wrong type of value is present. That's completely acceptable behavior, in my humble opinion. I'm just noticing different behavior from the version I was using before (which may have been quite old compared to the code in the debug-exception branch).

[solleer]

Yes, the parser is good about returning false for nonexistent values. When it does get messed up though is calling nonexistent functions.

[cxj]

That's a nice improvement. Is that change in master yet?

[solleer]

Yes, it was part of the new parser

[solleer]

@TomBZombie What do you thing of the debug-exception branch? The only thing left is having it return the Line number but I can't figure out a good way to do that.

[solleer]

It now tells the line of the error too. Had to change so stuff so that repeat would still work.

[TRPB]

Looking at the exceptions there's a bit of a performance overhead. I think we should use wrapper objects to do checks so you can turn on/off all the checks. However this will require use of DI throughout.

[cxj]

I think global on/off would be desirable. Using dependency injection (DI) throughout would also be nice.

I also realize it's a lot of work! Wish I could help, but having spent a few hours looking at the code, I have to say that at the moment, what it's doing is beyond me. I'm not grokking it yet.

[solleer]

Should I revert that commit

[solleer]

I think the change to the way the basedir works could help with caching though since the file name is passed around.

[TRPB]

Keep the commit and we'll use it as a basis. Here's what I'm thinking:

new Transphporm\Debug(tss, xml);

Runs a debugging mode by instantiating special versions of each property (so each module would have a special debug version) e.g. Module\Pseduo could work like this:

<?php
namespace Transphporm\Module\Debug;
class Pseudo implements \Transphporm\Module {

    public function load(\Transphporm\Config $config) {
        $config->registerPseudo(new \Transphporm\Pseudo\Attribute());
        $config->registerPseudo(new class implements \Transphporm\Pseudo {
            private $pseudo;

            public function __construct() {
                $this->pseudo = new \Transphporm\Pseudo\Nth();
            }

            public function match($name, $args) {
                if (!method_exists($this->pseudo, $args[0]) || !is_numeric($args[0])) {
                    throw new \Exception("Argument passed to 'nth-child' must be 'odd', 'even', or of type int");
                }
                $this->pseudo->match($name, $args);
            }
        });
        $config->registerPseudo(new \Transphporm\Pseudo\Not($config->getCssToXpath()));
    }
}

which would then do the checks if they were enabled by acting as a wrapper.

[solleer]

I think there should be a debug option in the Transphporm constructor that when enabled does what you talk about above as well as using the tss validator.

[TRPB]

Yeah, I think we need something optional, preferably a wrapper for Builder so it can easily be turned off and not impact performance in any way.

[TRPB]

Maybe we should have a try/catch that catches any error and then shows the error and runs the validator.

[solleer]

How would you use a wrapper if you type hint for Transphporm or want to substitute all instances of transphporm for the wrapper?

[solleer]

@TomBZombie Should debugging be a entirely different class or the third Boolean variable passed to the constructor?

[garrettw]

I shy away from Boolean parameters in general because they're not good for readability - unless you create class constants with a meaningful name that can be used for that same purpose.

[TRPB]

Really we need a debugger that is independent of transphporm. A wrapper works but it would be better to utilise the same parser but shouldn't really be used in place of a real transphporm instance. It should have the same API but just do error checking.

• Validate the syntax of the TSS file
• Check references to any included TSS or XML files
• Check references to data() lookups and provide relevant warnings if the data doesn't exist e.g. parse data(foo) and check that it actually exists.
• Check every CSS selector actually matches one or more elements. This should just provide a warning as we might be using @import for generic rules, it might be expected that they don't apply to every xml file

[solleer]

So would it have an output method that would return true if it validates or false if it doesn't?

[cxj]

An independent validator and checker sounds very much like a Linter to me -- and an excellent idea.

[solleer]

@TomBZombie What if we had a generic interface that Transphporm and the Debugger implemented called Level2\View or something similar?

[TRPB]

I briefly started working on a basic syntax validator yesterday. Here's what I have so far:

<?php
namespace Transphporm\Debug;
use Transphporm\Parser\Tokenizer;
use Transphporm\Exception;

class TssSyntaxCheck {
	private $tss;
	private $file;

	public function __construct($tss) {
		$this->tss = is_file($tss) ? file_get_contents($tss) : $tss;

		$this->file = is_file($tss) ? $tss : 'hardcoded tss string';
	}

	public function validate() {

		$tokens = (new Tokenizer($this->tss))->getTokens();
		$this->checkBraces($tokens);

		return true;
	}


	private function checkBraces($tokens) {
		foreach ($tokens as $token) {
			if ($token['type'] == Tokenizer::OPEN_BRACE) {
				foreach ($token['value'] as  $nestedToken)  {
					if ($nestedToken['type'] == Tokenizer::OPEN_BRACE) {
						$this->error('Braces cannot be nested', $token['line'], $token['string']);
					}
				}
			}
		}
	}

	private function error($text, $line = null, $near = null) {
		throw new Exception('TSS Parse Error: ' .$text, $this->file, $line, $near);
	}
}

It's hardly anything yet but by using the existing tokenizer, we can check for errors. Currently it just checks there are no nested braces but we can also check:

• Property names are followed by colons
• Property values are followed by a semicolon (unless it's the last rule)
• Properties are registered in the builder instance (will require some changes to expose internals somehow, and the error checker will need to instantiate or access a builder instance)
• Take the selector and pass it to CssToXpath to have that check for potential errors

I don't think we necessarily need them to have the same API. I think we can include a few debugging tools, but trigger them when an error occurs by using try/catch. Instead of checking for errors while parsing the TSS, we'll let the parser run into trouble. If it does, then performance isn't an issue and we can trigger all kinds of error checks in the catch block to work out what went wrong.

[solleer]

This is something I worked on before the update

namespace Transphporm;
class Debug {
    private $template;
	private $tss;
    private $filePath;
    private $validator;

    public function __construct($template, $tss = '') {
        $this->filePath = new FilePath();
        $this->validator = new TSSValidator();
		$this->template = $template;
        if (is_file($tss)) {
			$this->filePath->addPath(dirname(realpath($tss)));
			$tss = file_get_contents($tss);
		}
		$this->tss = (new SheetTokenizer($tss))->getTokens();
	}

    public function output($data = null, $document = false) {
        if (!$this->validator->validate($this->tss)) return false;

        return true;
    }

    private function checkFileRefrences($tss) {
        foreach (new TokenFilterIterator($this->tss, [Tokenizer::WHITESPACE]) as $token) {
            if (!$this->checkImportFile($token)) return false;
        }
        return true;
    }

    private function checkImportFile($token) {
        if ($token['type'] !== Tokenizer::AT_SIGN) return false;
        $tokens = $this->tss->from(Tokenizer::AT_SIGN, false)->to(Tokenizer::SEMI_COLON, false);
		$funcName = $tokens->from(Tokenizer::NAME, true)->read();
		$args = $this->valueParser->parseTokens($tokens->from(Tokenizer::NAME));
        return is_file($args[0]);
    }
}