Deprecate redundant HTTP keys

[paulbalandan]

Description
With the merge of the refactored Security class, the redundant HTTP.disallowedAction and HTTP.invalidSamSiteSetting can be removed in favor of the Security counterpart.

Checklist:

• Securely signed commits

[sfadschm]

I just checked and they still seem to be in use, e.g., here:

CodeIgniter4/system/HTTP/Exceptions/HTTPException.php

Lines 242 to 245 in a2de1f6

	public static function forInvalidSameSiteSetting(string $samesite)
	{
	return new static(lang('HTTP.invalidSameSiteSetting', [$samesite]));
	}

and in the corresponding tests.
I'm all for dropping as many duplicates as possible but not sure if it makes sense to change HTTP to Security in the above example.

[sfadschm]

Can we maybe add some testing for all language strings called in the framework to see if they are really defined?

[MGatner]

I like @sfadschm's idea, it could even be implemented as an Action for PRs. It should check both directions: for language calls missing a key and for unused keys.

For this PR, and generally, I think that we need to deprecate language strings instead of removing them because any module or project could be using the framework strings. While it technically would not crash the code the output could suddenly change to something unhelpful like "System.description"

[sfadschm]

Checking both directions sounds reasonable. I thought about such tests some days ago, but the only thing I could come up with was to load all language keys, then read the contents of all framework files and preg_match_all all the contents with lang('...').
The results could then be generated with two simple array_diffs but I still feel this puts some significant extra load on the tests.

I agree on the deprecation, but I think we can not really have both tests and deprecation, as the tests would have no way to tell a language string has already been deprecated and should not throw an error, or am I missing out on something?

[paulbalandan]

I believe system messages should be strictly for internal use and should not be used in users' code and covered by BC promise. The usage should remain strictly in the framework alone and users should implement their own keys. Since these messages are typically used within thrown exceptions, users wanting to use the messages should use it rather indirectly, i.e. using the throw Exception::something();

[sfadschm]

I believe system messages should be strictly for internal use and should not be used in users' code and covered by BC promise. The usage should remain strictly in the framework alone and users should implement their own keys. Since these messages are typically used within thrown exceptions, users wanting to use the messages should use it rather indirectly, i.e. using the throw Exception::something();

I'm with you on this, but I still also think that there is projects out there using those messages, which will face some unexpected messages like MGatner said.

How about applying this to 4.1 with a note in the update changelog and maybe a small notice in the user manual, that these keys are not BC covered?

[MGatner]

@paulbalandan What do you think of @sfadschm's idea? I think that is a good compromise: we add a comment that these are deprecated but leave them, then note in the UG and changelog for 4.1 the system language strings are all considered internal and can be changed without warning.

@lonnieezell I think you should weigh in on this as well.

[lonnieezell]

I agree system messages are intended to be for internal use. I like the suggested compromise.

[MGatner]

Key deprecation looks good. The PHP 8 SA is allowed (#4065). What about a line in the UG and changelog about language files being internal? or were you thinking that would be a separate PR?

[paulbalandan]

Key deprecation looks good. The PHP 8 SA is allowed (#4065). What about a line in the UG and changelog about language files being internal? or were you thinking that would be a separate PR?

Yes, I'm thinking of a separate PR for them.