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

statement_indentation with stick_comment_to_next_continuous_control_statement isn't honored? #7838

Open
tenzap opened this issue Feb 14, 2024 · 3 comments

Comments

@tenzap
Copy link

tenzap commented Feb 14, 2024

Bug report

Description

With the attached config, the comment is not indented correctly.
Input is correct, output gets modified in a wrong way.

Runtime version

PHP CS Fixer 3.49.0 Insomnia by Fabien Potencier, Dariusz Ruminski and contributors.
PHP runtime: 8.2.7

Used command

utils/vendor/bin/php-cs-fixer fix -v --show-progress=dots --verbose -vvv

Configuration file

Referenced file : finder.inc.php only contains inclusion & exclusion of paths.

Line in cause:

      'statement_indentation' => ['stick_comment_to_next_continuous_control_statement' => true],
<?php

# Get $finder variable
include_once 'utils/php-cs-fixer-configs/finder.inc.php';

$config = new PhpCsFixer\Config();
return $config
    ->setIndent("\t") // As per CI3 coding style
    ->setLineEnding("\n") // As per CI3 coding style
    ->setRules([
        // 0-spaces
        'encoding' => true,
        'indentation_type' => true,
        'line_ending' => true,
        'no_trailing_whitespace' => true,
        'no_whitespace_in_blank_line' => true,
        'single_blank_line_at_eof' => true,
        'no_trailing_whitespace_in_comment' => true,

        'array_indentation' => true,
        'no_whitespace_before_comma_in_array' => true,
        'whitespace_after_comma_in_array' => true,
        'trim_array_spaces' => true,
        'no_spaces_around_offset' => true,
        'no_blank_lines_after_class_opening' => false,

        # 1
        'single_quote' => true,

        # 2


        # 3
        'constant_case' => [ 'case' => 'upper'], //TRUE, FALSE...

        # 4
        //'single_space_around_construct' => true,
        'control_structure_braces' => true,
        'control_structure_continuation_position' => [ 'position' => 'next_line'],
        'declare_parentheses' => true,
        'no_multiple_statements_per_line' => true,
        'braces_position' => [ 'control_structures_opening_brace' => 'next_line_unless_newline_at_signature_end' ,
                            'anonymous_functions_opening_brace' => 'next_line_unless_newline_at_signature_end',
                            'anonymous_classes_opening_brace' => 'next_line_unless_newline_at_signature_end',
                            'allow_single_line_empty_anonymous_classes' => false,
                            'allow_single_line_anonymous_functions' => false,
                            'classes_opening_brace' => 'same_line',
                             ],
        'statement_indentation' => ['stick_comment_to_next_continuous_control_statement' => true],

        # 5
        //'strict_comparison' => true, // Remplace == by === etc... --> RISKY

        # 6
        'method_argument_space' => ['on_multiline' => 'ensure_fully_multiline'],

        # 7
        'explicit_string_variable' => true,

        # 8
        'no_closing_tag' => true,

        # 9
        'linebreak_after_opening_tag' => true,

        # 10
        'single_line_comment_style' => true,

        # 11
        'not_operator_with_space' => true,
        'spaces_inside_parentheses' => true, # Use with not_operator_with_space to be sure "( ! $var)" remains correct.
        'object_operator_without_whitespace' => true,
        'operator_linebreak' => [ 'only_booleans' => true ],
        'standardize_not_equals' => true,
        'ternary_operator_spaces' => true,
        'unary_operator_spaces' => true,
        'binary_operator_spaces' => true,

        # 12
        // 'no_alternative_syntax' => true, //Don't use it here because we want to keep alternative syntax on views

    ])
    ->setFinder($finder)

;

Code snippet that reproduces the problem

<?php
class Messages extends MY_Controller {

	function compose_process()
	{

		// Select send option
		switch ($this->input->post('sendoption'))
		{
			// Phonebook
			case 'sendoption1':
				$tmp_dest = explode(',', $this->input->post('personvalue'));
				foreach ($tmp_dest as $key => $tmp)
				{
					if (trim($tmp) !== '')
					{
						list ($id, $type) = explode(':', $tmp);
						// Person
						if ($type === 'c')
						{
							// Already sent, no need to send again
							if (in_array($id, $dest))
							{
								continue;
							}
							$dest[] = $id;
						}
						// Group
						else
						{
							if ($type === 'g')
							{
								$param = array('option' => 'bygroup', 'group_id' => $id);
								foreach ($this->Phonebook_model->get_phonebook($param)->result() as $group)
								{
									// Already sent, no need to send again
									if (in_array($group->Number, $dest))
									{
										continue;
									}
									$dest[] = $group->Number;
								}
							}
							// User, share mode
							else
							{
								if ($type === 'u')
								{
									// set share user id, process later
									$share_uid[] = $id;
								}
							}
						}
					}
				}
				break;

			// Input manually
			case 'sendoption3':
				$tmp_dest = explode(',', $this->input->post('manualvalue'));
				foreach ($tmp_dest as $key => $tmp)
				{
					$this->_phone_number_validation($tmp);
					$dest[$key] = phone_format_e164($tmp);
				}
				break;

			// Import from file  (CSV)
			case 'sendoption4':
				if (intval($this->input->post('import_value_count')) > 0)
				{
					$tmp_dest = explode(',', $this->input->post('Number'));
					foreach ($tmp_dest as $key => $tmp)
					{
						$dest[$key] = phone_format_e164($tmp);
					}
				}
				break;

			// Reply
			case 'reply':
				$dest[] = $this->input->post('reply_value');
				break;

			// Resend
			case 'resend':
				$dest[] = $this->input->post('resend_value');
				break;

			// Member
			case 'member':
				$this->load->model('sms_member/sms_member_model', 'Member_model');
				foreach ($this->Member_model->get_member('all')->result() as $tmp)
				{
					$dest[] = $tmp->phone_number;
				}
				break;

			// Phonebook group
			case 'pbk_groups':
				$param = array('option' => 'bygroup', 'group_id' => $this->input->post('id_pbk'));
				foreach ($this->Phonebook_model->get_phonebook($param)->result() as $tmp)
				{
					$dest[] = $tmp->Number;
				}
				break;

			// All contacts
			case 'all_contacts':
				$param = array('option' => 'all');
				foreach ($this->Phonebook_model->get_phonebook($param)->result() as $tmp)
				{
					$dest[] = $tmp->Number;
				}
				break;
		}
	}
}
@Wirone
Copy link
Member

Wirone commented Feb 14, 2024

Please provide minimal reproducer that don't refer to files from your filesystem. This should be a simple snippet + command like ./php-cs-fixer check --verbose --diff reproducer.php --rules='{"statement_indentation":{"stick_comment_to_next_continuous_control_statement":true}}', so it's possible to verify it without modifying local config.

Anyway, I was able to verify that \t indentation is not a problem here, since with 4-space indentation the problem is the same:

image

@keradus you know the most about this option, maybe you can check what happens here 🙂.

@julienfalque
Copy link
Member

This would likely be fixed by #6490.

Copy link

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants