Skip to content

[GH 8420] PHP Constructor code completion should not add [#Override] #8421

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

NReib
Copy link
Contributor

@NReib NReib commented Apr 13, 2025

Override Attribute on constructors in PHP leads to compile errors (See also #8334)

  • No longer add override on constructors

before:
before_01

after:
after_01


^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

@NReib
Copy link
Contributor Author

NReib commented Apr 13, 2025

I only added a test for SelectedPropertyMethodsCreator - I did not find any tests for the PHPCompletionItem class.

It would be nice if this could still be added in the next version since this behaviour creates annoying bugs.

@NReib
Copy link
Contributor Author

NReib commented Apr 13, 2025

found it - will add some more tests

@NReib NReib force-pushed the BOverrideOnConstructor branch from d3a44ed to e17a2a3 Compare April 13, 2025 12:02
@NReib
Copy link
Contributor Author

NReib commented Apr 13, 2025

Added more tests.
I found in the added test that all magic methods are duplicated - a) with Override and b) without override. Do we want to filter out b) if a) is included in the resultset?

@troizet troizet added the PHP [ci] enable extra PHP tests (php/php.editor) label Apr 13, 2025
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

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

Please add a white space after if. Thanks.

Override Attribute on constructors in PHP leads to compile errors
(See also apache#8334)

- No longer add override on constructors
@NReib NReib force-pushed the BOverrideOnConstructor branch from e17a2a3 to ad3d9c6 Compare April 13, 2025 15:33
@junichi11
Copy link
Member

Please separate screenshots before & after.

Before:
[before image]

After:
[after image]

@junichi11 junichi11 added this to the NB26 milestone Apr 13, 2025
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

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

Thank you for your work!

@junichi11 junichi11 linked an issue Apr 14, 2025 that may be closed by this pull request
@junichi11 junichi11 merged commit b9d00b8 into apache:master Apr 14, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PHP] Constructor code completion should not add [#Override]
3 participants