Extension system

[shyim]

Hey,

we're currently adding custom checks into this tool using composer patches. I feel really dirty to add a custom check into the list of https://github.com/Roave/BackwardCompatibilityCheck/blob/6.1.x/bin/roave-backward-compatibility-check.php#L86

I guess a simple extension system could help here already to add custom checks.

What I think is:

• A interface with methods like getClassChecks getMethodChecks... which returns the instances
• The implemented class can be provided using extra part in the composer.json
• We use composer to look them up to register them in the roave-backward-compatibility-check.php or maybe move the core things to the same interface based thingy?

[Ocramius]

This block of code is literally the configuration of this utility:

BackwardCompatibilityCheck/bin/roave-backward-compatibility-check.php

Lines 82 to 311 in d4c84f0

	new CompareClasses(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ExcludeAnonymousClasses(new ClassBased\ExcludeInternalClass(
	new ClassBased\MultipleChecksOnAClass(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameAbstract()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameInterface()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameTrait()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameFinal()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\PropertyRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\AncestorRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameInternal()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\OpenClassChanged(
	new ClassBased\MultipleChecksOnAClass(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantChanged(
	new ClassConstantBased\MultipleChecksOnAClassConstant(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\OnlyPublicClassConstantChanged(
	new ClassConstantBased\MultipleChecksOnAClassConstant(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantVisibilityReduced()),
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantValueChanged())
	)
	)),
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\OnlyProtectedClassConstantChanged(
	new ClassConstantBased\MultipleChecksOnAClassConstant(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantVisibilityReduced()),
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantValueChanged())
	)
	))
	)
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\PropertyChanged(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\ExcludeInternalProperty(new PropertyBased\MultipleChecksOnAProperty(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\OnlyPublicPropertyChanged(
	new PropertyBased\MultipleChecksOnAProperty(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyBecameInternal()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyTypeChanged(new TypeIsContravariant(), new TypeIsCovariant())),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyDefaultValueChanged()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyVisibilityReduced()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyScopeChanged())
	)
	)),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\OnlyProtectedPropertyChanged(
	new PropertyBased\MultipleChecksOnAProperty(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyBecameInternal()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyTypeChanged(new TypeIsContravariant(), new TypeIsCovariant())),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyDefaultValueChanged()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyVisibilityReduced()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyScopeChanged())
	)
	))
	)))
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodChanged(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\ExcludeInternalMethod(new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\OnlyPublicMethodChanged(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodBecameFinal()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
	new FunctionBased\MultipleChecksOnAFunction(
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\RequiredParameterAmountIncreased()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterDefaultValueChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
	)
	))
	)
	)),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\OnlyProtectedMethodChanged(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodBecameFinal()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
	new FunctionBased\MultipleChecksOnAFunction(
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\RequiredParameterAmountIncreased()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterDefaultValueChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
	)
	))
	)
	))
	)))
	))
	)
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\FinalClassChanged(
	new ClassBased\MultipleChecksOnAClass(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantChanged(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\OnlyPublicClassConstantChanged(
	new ClassConstantBased\MultipleChecksOnAClassConstant(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantVisibilityReduced()),
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantValueChanged())
	)
	))
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\PropertyChanged(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\OnlyPublicPropertyChanged(
	new PropertyBased\MultipleChecksOnAProperty(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyBecameInternal()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyTypeChanged(new TypeIsContravariant(), new TypeIsCovariant())),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyDefaultValueChanged()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyVisibilityReduced()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyScopeChanged())
	)
	))
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodChanged(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\OnlyPublicMethodChanged(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodBecameFinal()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
	new FunctionBased\MultipleChecksOnAFunction(
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\RequiredParameterAmountIncreased()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterDefaultValueChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant()))
	)
	))
	)
	))
	))
	)
	))
	)
	))),
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\ExcludeInternalInterface(new InterfaceBased\MultipleChecksOnAnInterface(
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\InterfaceBecameClass()),
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\InterfaceBecameTrait()),
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\AncestorRemoved()),
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\MethodAdded()),
	new InterfaceBased\SkipInterfaceBasedErrors(new InterfaceBased\UseClassBasedChecksOnAnInterface(
	new ClassBased\MultipleChecksOnAClass(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameInternal()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantChanged(
	new ClassConstantBased\SkipClassConstantBasedErrors(new ClassConstantBased\ClassConstantValueChanged())
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodChanged(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
	new FunctionBased\MultipleChecksOnAFunction(
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\RequiredParameterAmountIncreased()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterDefaultValueChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
	)
	))
	)
	))
	)
	))
	))),
	new TraitBased\SkipTraitBasedErrors(new TraitBased\ExcludeInternalTrait(new TraitBased\MultipleChecksOnATrait(
	new TraitBased\SkipTraitBasedErrors(new TraitBased\TraitBecameInterface()),
	new TraitBased\SkipTraitBasedErrors(new TraitBased\TraitBecameClass()),
	new TraitBased\SkipTraitBasedErrors(new TraitBased\UseClassBasedChecksOnATrait(
	new ClassBased\MultipleChecksOnAClass(
	new ClassBased\SkipClassBasedErrors(new ClassBased\ClassBecameInternal()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\ConstantRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\PropertyRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodRemoved()),
	new ClassBased\SkipClassBasedErrors(new ClassBased\PropertyChanged(
	new PropertyBased\MultipleChecksOnAProperty(
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyBecameInternal()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyTypeChanged(new TypeIsContravariant(), new TypeIsCovariant())),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyDefaultValueChanged()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyVisibilityReduced()),
	new PropertyBased\SkipPropertyBasedErrors(new PropertyBased\PropertyScopeChanged())
	)
	)),
	new ClassBased\SkipClassBasedErrors(new ClassBased\MethodChanged(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\MultipleChecksOnAMethod(
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodBecameFinal()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodConcretenessChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodScopeChanged()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodVisibilityReduced()),
	new MethodBased\SkipMethodBasedErrors(new MethodBased\MethodFunctionDefinitionChanged(
	new FunctionBased\MultipleChecksOnAFunction(
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\FunctionBecameInternal()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeByReferenceChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\RequiredParameterAmountIncreased()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterDefaultValueChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeCovarianceChanged(new TypeIsCovariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ReturnTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeContravarianceChanged(new TypeIsContravariant())),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterTypeChanged()),
	new FunctionBased\SkipFunctionBasedErrors(new FunctionBased\ParameterNameChanged())
	)
	))
	)
	)
	))
	)
	))
	)))
	)
	);

The current way you customize it is by copying the whole roave-backward-compatibility-check.php and writing your own version of it. The piece that is usually "in common" is this:

BackwardCompatibilityCheck/bin/roave-backward-compatibility-check.php

Lines 55 to 81 in d4c84f0

	$application = new Application('roave/backward-compatibility-check', Versions::getVersion('roave/backward-compatibility-check'));
	$helperSet = $application->getHelperSet();
	$input = new ArgvInput();
	$output = new ConsoleOutput();
	$astLocator = (new BetterReflection())->astLocator();
	$composerIo = new ConsoleIO($input, $output, $helperSet);
	$apiCompareCommand = new Command\AssertBackwardsCompatible(
	new GitCheckoutRevisionToTemporaryPath(),
	new ComposerInstallationReflectorFactory(new LocateSourcesViaComposerJson($astLocator)),
	new GitParseRevision(),
	new GetVersionCollectionFromGitRepository(),
	new PickLastMinorVersionFromCollection(),
	new LocateDependenciesViaComposer(
	static function (string $installationPath) use ($composerIo): Installer {
	return Installer::create(
	$composerIo,
	(new Factory())->createComposer(
	$composerIo,
	null,
	true,
	$installationPath
	)
	);
	},
	$astLocator
	),

Even then, it is not uncommon that the from/to source locators are customized by end-users.

Requiring vendor/autoload.php is generally a one-liner, in custom projects:

BackwardCompatibilityCheck/bin/roave-backward-compatibility-check.php

Lines 38 to 53 in d4c84f0

	(static function (): void {
	$autoloaderLocations = [
	__DIR__ . '/../vendor/autoload.php', // Installed by cloning the project and running `composer install`
	__DIR__ . '/../../../autoload.php', // Installed via `composer require`
	];
	foreach ($autoloaderLocations as $autoload) {
	if (file_exists($autoload)) {
	require_once $autoload;
	return;
	}
	}
	throw new RuntimeException('Could not find Composer autoload.php');
	})();

I thought about how to make this "easier to customize", but in practice, it is better to:

1. copy the file
2. adjust as needed
3. have it covered by your own static analysis checks, which will pick up any BC breaks/incompatibilities

So don't feel dirty about it: it's still the best way forward, and if you needed to change one tiny check 10 levels deep in the structure, you still have to rewrite the whole instantiation chain anyway :)

One simplification I thought of is unifying bin/roave-backward-compatibility-check.php and bin/roave-backward-compatibility-check, but need to check if the non-.php file can still be statically analyzed.