Add strict rules

[VincentLanglet]

No description provided.

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[VincentLanglet]

@jordisala1991 What do you recommend in order to solve

return $request->isXmlHttpRequest() || $request->get('_xml_http_request');

?

Changing to

return $request->isXmlHttpRequest() || true === $request->get('_xml_http_request');

could stop working if the get wasn't returning a boolean. I don't know what was returned before.

An option could be to use

$request->attributes->getBoolean('_xml_http_request')

but is it supposed to be passed as attributes, query or request ?
I dont like checking

$request->attributes->getBoolean('_xml_http_request') || $request->request->getBoolean('_xml_http_request') || $request->query->getBoolean('_xml_http_request')

cc @sonata-project/contributors

[jordisala1991]

@jordisala1991 What do you recommend in order to solve

return $request->isXmlHttpRequest() || $request->get('_xml_http_request');

?

Changing to

return $request->isXmlHttpRequest() || true === $request->get('_xml_http_request');

could stop working if the get wasn't returning a boolean. I don't know what was returned before.

An option could be to use

$request->attributes->getBoolean('_xml_http_request')

but is it supposed to be passed as attributes, query or request ?

I dont like checking

$request->attributes->getBoolean('_xml_http_request') || $request->request->getBoolean('_xml_http_request') || $request->query->getBoolean('_xml_http_request')

cc @sonata-project/contributors

We need to check where it is passed that parameter, it will probably be query if it is a get call or request if it is a post

[VincentLanglet]

We need to check where it is passed that parameter, it will probably be query if it is a get call or request if it is a post

In tests, we're setting the attributes, but I think it's wrong.

Here, it's POST

SonataAdminBundle/src/Resources/views/CRUD/Association/edit_one_script.html.twig

Lines 49 to 51 in 2cce28c

	type: "POST",
	dataType: 'html',
	data: { _xml_http_request: true },

SonataAdminBundle/src/Resources/views/CRUD/Association/edit_many_script.html.twig

Lines 331 to 333 in 2cce28c

	data: {_xml_http_request: true },
	dataType: 'html',
	type: 'POST',

Here, it rely on the form

SonataAdminBundle/src/Resources/views/CRUD/Association/edit_many_script.html.twig

Lines 93 to 96 in 2cce28c

	type: form.attr('method'),
	url: form.attr('action'),
	dataType: 'html',
	data: {_xml_http_request: true},

Here it rely on an attr.method, or can be GET

SonataAdminBundle/src/Resources/views/CRUD/Association/edit_many_script.html.twig

Lines 259 to 296 in 2cce28c

	if (this.nodeName == 'FORM') {
	var url = element.attr('action');
	var type = element.attr('method');
	} else if (this.nodeName == 'A') {
	var url = element.attr('href');
	var type = 'GET';
	} else {
	alert('unexpected element : @' + this.nodeName + '@');
	return;
	}
	if (element.hasClass('sonata-ba-action')) {
	Admin.log('[{{ id }}|field_dialog_form_action] reserved action stop catch all events');
	return;
	}
	var data = {
	_xml_http_request: true
	}
	var form = jQuery(this);
	Admin.log('[{{ id }}|field_dialog_form_action] execute ajax call');
	var oldErrorMessages = jQuery(field_dialog_content_{{ id }}).find('div.alert-danger');
	// Remove old error messages.
	if (oldErrorMessages.length > 0) {
	oldErrorMessages.remove();
	}
	// the ajax post
	jQuery(form).ajaxSubmit({
	url: url,
	type: type,
	headers: {
	Accept: 'application/json'
	},
	data: data,

So I would say, it can be either GET or POST.

Do we have to check both query and request ?
(And what is attributes ^^')

[jordisala1991]

Cant some of these be removed or they conflict with other errors for psalm?

[VincentLanglet]

The issue is that we checking our return type.

for instance the method

function createDate(): \DateTime {
    return new \DateTime();
}

has the following test

self::assertInstanceOf(\DateTime::class, createDate());

Phpstan is complaining about this being always true. But we might consider it's the purpose of the test to be sure it will be true in the futur.

[jordisala1991]

missing message, same below

[VincentLanglet]

Indeed, I forgot ^^'