Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Add support for function arguments to be passed in the same line as the function #41

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

srtfisher
Copy link
Member

@srtfisher srtfisher commented Jul 18, 2024

Allows multi-line function calls to support passing arguments on the same line as the function name. For example, this is now valid code:

ai_method_call( [
	'foo' => 'bar',
] );

Previously had to only write this as (still supported though):

ai_method_call(
	[
		'foo' => 'bar',
	]
);

Disables the PEAR.Functions.FunctionCallSignature sniff entirely and replaces it with PSR2.Methods.FunctionCallSignature.

@srtfisher srtfisher requested a review from a team as a code owner July 18, 2024 17:57
@kingkool68
Copy link

Not a strong opinion here but for something as short as one line of arguments I would do

ai_method_call( [ 'foo' => 'bar' ] );

so not sure I would ever do

ai_method_call( [
	'foo' => 'bar',
] );

and

ai_method_call( [
	'foo' => 'bar',
    'baz' => 'bam',
] );

would be illegal so I don't really see the point

@srtfisher
Copy link
Member Author

srtfisher commented Jul 18, 2024

I would and have done this previously and ignored the rule:

ai_method_call( [
	'foo' => 'bar',
], true );

or more likely

ai_method_call( [
	'foo' => 'bar',
	'foo2' => 'bar',
	'foo3' => 'bar',
	'foo4' => 'bar',
] );

Your example wouldn't be illegal -- it'd be what I'm suggesting to make it.

Copy link

@mogmarsh mogmarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have run into this occasionally, but always just fix it rather than ignore it. I think sometimes this will be more readable, sometimes not... we'll trust the dev and/or the reviewer to figure that out. This gives us options.
👍

@srtfisher srtfisher enabled auto-merge July 19, 2024 19:38
@srtfisher srtfisher merged commit 242b93f into develop Jul 19, 2024
1 check passed
@srtfisher srtfisher deleted the feature/allow-array-same-line-function branch July 19, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants