Boolean

This logic will not work. An object being traversable does not mean it will always be handled through the adder and remover. This is even a very uncommon case (the only way I see to do it is to attach thecollection type on thegetIterator method). The adder and remover are on the parent object, not on the collection itself

Logic modified. I had read PropertyAccessor wrong. If there is an adder and a remover however, we want to assumesubmit is there.

IMO this should be moved to a static property:

privatestatic$nameToTypeMapping =array('submit' =>'submit','reset' =>'reset',);

@bschussek: This would prevent someone from modifying the list of supported mappings thru inheritance.

Consider this scenario: Someone creates their ownbutton type (let's call itrandom for example sakes) and they want to allow$builder->add('random')... The way it is now, they can easily overridegetSupportedButtonTypeMappings() in a way that is agnostic to future modifications of the mappings as such:

/** * {@inheritDoc} */protected function getSupportedButtonTypeMappings(){    $mappings = parent::getSupportedButtonTypeMappings();    $mappings['random'] = 'random';    return $mappings;}

and then change the%form.type_guesser.core.class% parameter to their overloaded class.

By making it aprivate static variable, the developer in the scenario above would have to redo all the work.

If they provide their own type, they should be providing a separate guesser, not replacing the guesser of the core extension

I'm sorry@stof but Istrongly disagree. The whole point of having an object-oriented service container is to be able to replace specific services with your own implementation that extends the base implementation if you have a very small change to make.In fact, this is one of the main advantages of dependency injection coupled with the service locator pattern.

The use case above is a perfectly valid use case that happens all the time in software development. Forcing the user to copy-paste code into its own implementation to simply add what arguably should be a configurable element is, IMHO, an idiotic design decision. Ping@fabpot

In fact, this is something I'm doing in my own projects. For example, I extended the defaultProfilerListener to only save profiling information of non-404 exceptions whenonly_exceptions is enabled in the configuration. IfonKernelException would not of been overridable and theprofiler_listener.class parameter would not be available, I would of had to roll out my own completeProfilerListener implementation. That would of been costly and not at all elegant. Plus, this approach would of required me to update it manually when the Symfony version updates. That would of been utterly ridiculous.

@FineWolf I'm not talking about the container here. I'm talking about the way the Form component works. Extending the core guesser to replace it can only be done by 1 package wanting to provide a new button type (the next one would need to be aware of the previous overwrite, which is impossible for shared stuff). The Form component lets many different guessers being registered, so registering a separate guesser is a fat better way to extend the form component than replacing the gusser of another extension.
Composition is most of the time a far better extension point than inheritance.

Your argument for making the class extendable is that other guesser may need to reuse the other protected properties to inspect the class. My own conclusion based on the same need is that these other protected methods should not be part of this type guesser but be in a collaborator instead (which can then be reused by other guessers)

@stof The point is not to make it extendable for multiple packages, but on a special case basis. Of course if you are building a reusable bundle you would want to roll out your ownTypeGuesser, however there are special cases where for when you are building that one specific project in that one specific case, you might want to add a new supported button type, or maybe disable the support forreset and keep the one for submit.

Both are totally valid points. However I do not think we must be overly restrictive in this case. There are still valid use-cases for having aprotected function here.

In fact, having itprotected here does allow someone to register their implementation as a new TypeGuesser, keeping the logic of the original one and simply returning a new supported type.

At the end of the day, I'll go with whatever you guys decide. Just make an informed decision.

The issue if we make it protected is that we have to maintain BC on it for the full 2.x lifetime (because changing a protected method is a BC rbeak fo people extending the class). This is why we use private everywhere unless we explicitly want to make it a supported extension point (and before switching a method to protected to add an extension point, we think about other ways to achieve the extension to decide whether switching to protected is the best way to provide it. As I said at the end of my command above, there might be better ways to achieve the use case)

@stof I'll move the property detection logic in a separate class (which would allow otherTypeGuessers to use it) and make array private static as asked.

run php-cs-fixer

It looks like much code duplication with the PropertyAccess component. We can probably reuse it which is a required dependency anyway. Maybe the newisReadable method, see#10570

@Tobion you should have looked at the discussion first. The PR was already referenced

Shouldn't this a regular method? As your creating a closure every time the camelize method is called (which can be quite allot).