Problem/Motivation

From the PHP manual:

By default, PHP will coerce values of the wrong type into the expected scalar type declaration if possible. For example, a function that is given an int for a parameter that expects a string will get a variable of type string.

This silent type-casting is often unintentional and can result in bugs.

We set declare(strict_types=1); in 28 of the 35 PHP files in the test directory (at time-of-writing). We should do this in all tests. While it only affects how PHP juggles types in the test, we should start somewhere!

Proposed resolution

Add declare(strict_types=1); to all test PHP files.

Remaining tasks

  1. Merge request for 9.0.x - merge request !205 created by @mparker17 in #3
  2. Merge request for 8.0.x - merge request !206 created by @mparker17 in #5
  3. Community review - skipped by @mparker17 in #7
  4. Maintainer review - skipped by @mparker17 in #7
  5. Commit 9.0.x - merged by @mparker17 in #9
  6. Commit 8.0.x - merged by @mparker17 in #11
  7. Release 9.0.x - released in 9.0.0-alpha3 by @mparker17
  8. Release 8.0.x - released in 8.0.0-alpha7 by @mparker17

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork elasticsearch_connector-3586367

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mparker17 created an issue. See original summary.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

mparker17 opened merge request !205

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Active » Needs review

Added a MR for 9.0.x; lets see what testbot thinks before we create something for 8.0.x.

mparker17 opened merge request !206

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

9.0.x tests pass, creating an 8.0.x MR

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

Great, tests pass in both branches.

Given that the change is to test-only code, it should be a very-low-impact change, so I'm going to skip the community review. And given that I'm the maintainer, it doesn't need a maintainer review step.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Status: Needs review » Reviewed & tested by the community

Should have moved to RTBC in the last comment.

  • mparker17 committed 1965641a on 9.0.x 
    feat: #3586367 Declare strict_types in all tests

By: mparker17
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Version: 9.0.x-dev » 8.0.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -needs backport to 8.0.x

Great! Merging to 8.0.x

  • mparker17 committed 848acd31 on 8.0.x 
    feat: #3586367 Declare strict_types in all tests

By: mparker17
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Patch (to be ported) » Fixed

Merged to 8.0.x.

I will update this issue when the code is released.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

The changes in this issue were released in elasticsearch_connector-9.0.0-alpha3, and elasticsearch_connector-8.0.0-alpha7

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.