Contrib.social

Undefined varibles in \Drupal\Core\Site\SettingsEditor

Open on Drupal.org →
Created on 3 October 2014, about 10 years ago
Updated 27 August 2024, 3 months ago

Problem/Motivation

Hi everyone.

First of all this is my first issue report identified at Amsterdam DrupalCon 2014.

In core/includes/install.inc I have identified several unidentified variables in the function drupal_rewrite_settings.
While these variables might(probably) get set through the process of the foreach loop the variables should still have some default values.

This mitigates false positives from static code analysis made by IDE's etc.

The first issue:

        case 'right_bracket':
            if ($value == ']') {
              if (isset($current[$index])) {

The index variable is not set outside the scope and not defined in the case 'right_bracket'.

Second issue:

          case 'candidate_right':
            if (_drupal_rewrite_settings_is_simple($type, $value)) {
              $value = _drupal_rewrite_settings_dump_one($current);
              // Unsetting $current would not affect $settings at all.
              unset($parent[$index]);

Again three variables is used without being defined first or outside the scope of the switch statement.
The three variables are $current, $index and $parent.

The relevant code was moved to \Drupal\Core\Site\SettingsEditor

Steps to reproduce

Proposed resolution

TBA

Remaining tasks

Decide how to correctly define the possibly undefined variables

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component
Install  →

Last updated 3 days ago

No maintainer
Created by

🇩🇰Denmark RonnieJ

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment almost 2 years ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    Still seems to be relevant but the bulk of the code was moved to SettingsEditor.php

    Tagged for novice for beginner developers.

  • Comment almost 2 years ago →
    🇮🇳India _utsavsharma

    Patch for 10.1.x.

  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India Aadhar_Gupta

    Applying fix for 10.1.x and fixing custom commands

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    Not sure what happened but that patch is full of out of scope changes. Can compare the files sizes between the two and see

  • Status changed to Active over 1 year ago
  • Comment over 1 year ago →
    🇳🇿New Zealand quietone

    According to #14, this is not a novice issue. I am removing the tag. Setting to active since this needs a re-think.

  • Comment over 1 year ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    We also need a new issue summary and title given this now has nothing to do with install.inc

    I think rather than focussing on the unused variables we should focus on the design of \Drupal\Core\Site\SettingsEditor and see if we can improve that. And as part of that solve the "variable might not be defined" warnings in the class.

    Also this really is a task and not a bug.

  • Issue was unassigned.
  • Comment 3 months ago →
    🇳🇿New Zealand quietone

    #3244570

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024