Support strict_vars globally in application config

[hschletz]

I want a notice whenever a view template tries to access an undefined view variable via $this->var. I know only 2 ways to achieve this:

1. Call $this->vars()->setStrictVars(true) in every template
2. Set up an event listener and run setStrictVars(true) recursively on the view model and all its children (taking into account that a model's variables may be stored as an array instead of Zend\View\Variables).

Zend\View\Variables even supports an option "strict_vars" via its constructor, but neither this option nor setStrictVars() appear to be used anywhere within the ZF code.

It would be nice to be able to set strict_vars globally in the application configuration and have the Variables constructor evaluate it.

[Ocramius]

@hschletz why not making it a small module that registers a listener, and that can be re-distributed and enabled/disabled?
Changing Variables#$strictVars can't really be done in 2.x as it looks like quite a BC break to me...

[hschletz]

The change would not break BC if the default remains unchanged. I'm talking about a new option "strict_vars" in the application config. Since this option does not exist yet (only if passed manually to the Variables object), it will not be present in any existing application code - it would be an entirely new feature.

I think view variables should be treated like any other variable - ignoring reference to an undefined variable is bad practice.The hassle of an external module for this essential feature looks like overkill to me.

[Ocramius]

I'm talking about a new option "strict_vars" in the application config. Since this option does not exist yet (only if passed manually to the Variables object), it will not be present in any existing application code - it would be an entirely new feature.

Yeah, for this path, I really suggest having a module that deals with it. It can be as minimal as EdpModuleLayouts

[hschletz]

Sure, my workaround is not much code, but it's a bit quirky because it adds some postprocessing to all view models, instead of having things set up correctly in the first place. Additionally, I want to keep external dependencies as minimal as possible. This functionality really should be provided by the ZF core which currently not only encourages writing bad code, but makes writing better code needlessly complicated.

I'm not exaggerating with the "bad code" thing: Once enabled, the notices revealed several previously undiscovered bugs in one of my projects.

[Ocramius]

I fully agree on the strictness here, it would indeed help, but I would really just change that default value in 3.x and have a debug module in 2.x

[Martin-P]

Interesting info about setStrictVars().

I did some testing by attaching a listener to ViewEvent::EVENT_RENDERER and I am able to set strict vars on the ViewModel returned by the controller and on the layout. However, this does not cover the render() view helper and partial() view helper. I know this is not a forum for coding issues, but I think this can be useful for the actual implementation to make sure all parts are covered. Any ideas on how to cover that?

<?php

namespace MyStrictViewVars;

use Zend\EventManager\EventInterface;
use Zend\ModuleManager\Feature;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\View\Variables as ViewVariables;
use Zend\View\ViewEvent;

class Module implements Feature\BootstrapListenerInterface
{
    /**
     * @var ServiceLocatorInterface
     */
    protected $serviceManager;

    /**
     * Bootstrap event listener
     *
     * @param EventInterface $event
     */
    public function onBootstrap(EventInterface $event)
    {
        $application    = $event->getApplication();
        $eventManager   = $application->getEventManager();

        $identifier = 'Zend\View\View';
        $event      = ViewEvent::EVENT_RENDERER;
        $callback   = array($this, 'setStrictVars');
        $priority   = 100;
        $eventManager->getSharedManager()->attach($identifier, $event, $callback, $priority);

        $serviceManager = $application->getServiceManager();
        $this->setServiceManager($serviceManager);
    }

    /**
     * Set strict vars on ViewModel
     *
     * @param EventInterface $event
     */
    public function setStrictVars(EventInterface $event)
    {
        $config = $this->getServiceManager()->get('Config');
        if (!isset($config['view_variables']['strict_vars'])) {
            return;
        }

        $useStrictVars = (bool) $config['view_variables']['strict_vars'];
        $viewModel     = $event->getModel();

        /**
         * If $variables is not instance of ViewVariables,
         * convert it to ViewVariables and overwrite $viewModel variables
         */
        $variables = $viewModel->getVariables();
        if (!$variables instanceof ViewVariables) {
            $viewVariables = new ViewVariables($variables);
            $viewModel->setVariables($viewVariables, true);
        }

        $viewModel->getVariables()->setStrictVars($useStrictVars);
    }

    /**
     * @param ServiceLocatorInterface $serviceManager
     */
    protected function setServiceManager(ServiceLocatorInterface $serviceManager)
    {
        $this->serviceManager = $serviceManager;
    }

    /**
     * @return ServiceManager
     */
    protected function getServiceManager()
    {
        return $this->serviceManager;
    }
}

[Ocramius]

@Martin-P the solution there would be to just replace the PhpRenderer IMO.

[Martin-P]

Did some digging. The creation of PhpRenderer which is used in the application is hardcoded in the ViewManager on line 184 and the factory for the ViewManager is hardcoded in the ServiceListenerFactory on line 57. The ServiceListener is created even before the modules are loaded in the ModuleManagerFactory on line 44. It is possible to overwrite the ServiceListener via application.config.php:

return array(
    'service_manager' => array(
        'factories' => array(
            'ServiceListener' => 'ModuleName\Factory\ServiceListenerFactory',
        ),
    ),
    // more config
);

But now the namespace is still unknown, because the module is not yet loaded. Perhaps the dirty way by overriding the protected property $renderer of the ViewManager with Reflection? 😉

Edit: nevermind, this is becoming a mess 😃

[Martin-P]

Always funny to read back your own comments 😄
Easy solution: indeed overwrite PhpRenderer and set HelperPluginManager and Resolver from the already existing PhpRenderer. Overwrite the methods get, __get and vars and set strict mode before they are called. Add an initializer to the view_helpers config and inject your custom PhpRenderer with the setView() method of the view helper.

[antiphp]

why not making it a small module

Why not making it a small configuration option? :)

There is no reason to handle view variables different than regular variables, if they are not set but used.

[hschletz]

@GeeH: Still relevant, don't close. All available/proposed solutions are far too complicated for such a basic code quality feature. @antiphp has it summed up pretty well. I'd also suggest making the desired behavior the default for ZF3.

[grizzm0]

This sounds like the job of a module to me.

[GeeH]

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html
New issue can be found at: zendframework/zend-view#76