Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

3.5.25: Keine Namespace-Deklaration in der DCA möglich #8682

Closed
Babelfisch opened this issue Mar 27, 2017 · 15 comments
Closed

3.5.25: Keine Namespace-Deklaration in der DCA möglich #8682

Babelfisch opened this issue Mar 27, 2017 · 15 comments
Assignees
Labels
Milestone

Comments

@Babelfisch
Copy link

Nach dem Update auf die 3.5.25 und vermutlich dieser Änderung #8635, kann ich keine eigenen Namespaces mehr in der DCA angeben. Beispiel:

<?php 

namespace Mein\Namespace;

Fehler:

Fatal error: Cannot mix bracketed namespace declarations with unbracketed namespace declarations in /contao/system/cache/dca/tl_my_table.php on line 5

Nutze ich die Klammern-Schreibweise für Namespaces kommt dann dieser Fehler:

Fatal error: Namespace declarations cannot be nested in /contao/system/cache/dca/tl_my_table.php on line 5

Das sprengt jetzt eine ganze Reihe meiner Erweiterungen. :-(

@leofeyer
Copy link
Member

Das ist in der Tat nicht gut. @contao/developers /cc

@ausi
Copy link
Member

ausi commented Mar 27, 2017

Das hat zuvor vermutlich nur dann funktioniert wenn es für den betroffenen DCA nur eine einzige PHP-Datei gibt, sprich eine eigene Tabelle/Erweiterung, liege ich da richtig @Babelfisch?

@leofeyer Könnten wir eventuell auf den Fall prüfen, dass wenn es nur um eine einzige Datei geht, kein Namespace-Block hinzugefügt wird?

@Babelfisch
Copy link
Author

Ja, ich habe das in eigenen Tabellen verwendet.

@Toflar
Copy link
Member

Toflar commented Mar 27, 2017

Darf ich fragen, warum man einen Namespace in einer DCA Datei definieren will?

@leofeyer
Copy link
Member

leofeyer commented Mar 27, 2017

@leofeyer Könnten wir eventuell auf den Fall prüfen, dass wenn es nur um eine einzige Datei geht, kein Namespace-Block hinzugefügt wird?

Ja, aber löst das das Problem? In der Cache-Datei wäre ja dann ein Namespace-Deklaration mitten im Dokument und das frisst der PHP-Compiler auch nicht.

<?php

namespace / {
    // core
}

namespace Mein\Namespace;

// extension code

namespace / {
    // andere extension
}

@Toflar
Copy link
Member

Toflar commented Mar 27, 2017

Korrekt. Deswegen meine Frage. Das macht überhaupt keinen Sinn für mich. Klassen gehören nicht in eine DCA-Datei und deswegen weiss ich auch nicht warum man einen Namespace deklarieren möchte. Man kann welche useen aber deklarieren?

@Babelfisch
Copy link
Author

@Toflar: Na ja, bisher hat ja nichts dagegen gesprochen und da ich bei mir konsequent mit Namespaces gearbeitet habe, hab ich das auch in der DCA durchgezogen. Ist sicherlich nicht überall notwendig aber ein Minor-Release mit so einem BC-break ist auch nicht toll.

@Toflar
Copy link
Member

Toflar commented Mar 27, 2017

Na ja, bisher hat ja nichts dagegen gesprochen

Naja, doch eben eigentlich schon. Beim Aufbau des internen Caches würde es crashen, hättest du eine zweite Datei für dieselbe Tabelle was ja eher die Regel als die Ausnahme ist. Es ist also generell eigentlich nicht zu empfehlen weil es immer nur dann funktioniert, wenn nur genau eine Datei für einen DCA zuständig ist. Da man das als Extension-Entwickler nie wissen kann, fallen also per se alle Erweiterungen im ER/Composer weg. Es wäre ausschliesslich für applikationsspezifischen Code erlaubt und ich weiss nicht wie sinnvoll es ist, da ein Ausnahme-Handling einzubauen. Es würde dadurch ja quasi eine "bad practice" befürwortet.

@aschempp
Copy link
Member

@Toflar das stimmt nicht ganz, und nach meinem Wissen wurde das bisher so gehandhabt. Ein Namespace kann Klammern haben, folgendes ist valid:

namespace {
    // global stuff
}

namespace Contao {
    // Contao stuff
}

@Toflar
Copy link
Member

Toflar commented Mar 27, 2017

Das ist valid, hat keiner was anderes behauptet. Es funktioniert aber nicht, wenn Dateien zusammengeführt werden. Du darfst in einer DCA-Datei einfach keine Namespaces definieren. Wir könnten, wenn genau eine Datei einen definiert, den als namespace <xy> {} nehmen. Aber eben. Aufwand/Ertrag stimmt für mich nicht mal ansatzweise.

@ausi
Copy link
Member

ausi commented Mar 27, 2017

@leofeyer Könnten wir eventuell auf den Fall prüfen, dass wenn es nur um eine einzige Datei geht, kein Namespace-Block hinzugefügt wird?

Ja, aber löst das das Problem? In der Cache-Datei wäre ja dann ein Namespace-Deklaration mitten im Dokument und das frisst der PHP-Compiler auch nicht.

Es löst das Problem nicht vollständig, mehrere Dateien würden nach wie vor nicht funktionieren, das war aber auch vor unserer Änderung der Fall. Bei nur einer einzigen DCA-Datei pro Tabelle würde es aber wieder funktionieren und somit hätten wir den BC break behoben.

Ich seh es aber auch wie @Toflar, dass ich nicht sicher bin ob der Aufwand dafür steht, zumal man die DCA-Klasse ja auch einfach in eine eigene Datei verschieben kann ohne großen Aufwand.

@leofeyer
Copy link
Member

leofeyer commented Mar 27, 2017

Ich sehe es genauso, nur ist es halt schon so, dass es sich hierbei um einen BC-Break handelt, den wir irgendwie wieder beheben müssen. Ich sehe drei Möglichkeiten:

  1. Revert der Änderungen, wodurch wir das Problem aus Internal cache breaks for multiple extensions using the same classes in DCA files #8635 wieder hätten.

  2. Hinzufügen der namespace {}-Deklaration nur dann, wenn mindestens zwei Dateien zusammengefügt werden. Das würde funktionieren, ist aber ein häßlicher Workaround und löst nicht das eigentliche Problem.

  3. Such nach namespace …;-Deklarationen und Umwandlung in namespace … {}-Deklarationen vor dem Zusammenfügen der Dateien. Und natürlich kein Wrapping, wenn schon ein Wrapper besteht.

Nummer 3 wäre vermutlich die sauberste Lösung.

@ausi
Copy link
Member

ausi commented Mar 27, 2017

Variante 3 könnte vermutlich ähnlich funktionieren wie das declare()-Handling in contao/core-bundle#730 von @Toflar.

@leofeyer leofeyer added this to the 3.5.26 milestone Mar 29, 2017
@backbone87
Copy link
Contributor

ich bin für variante 3 und zwar pauschal, egal ob nur eine datei oder mehrere zusammen gefasst werden, oder ob namespace ; oder namespace {} verwendet wird.

ein pseudo algorithmus könnte so aussehen:

  • wenn in der datei kein namespace verwendet wird -> wrappe die datei in einen namespace { .. } block (global namespace block)
  • wenn in der datei ein oder mehrere namespace ; verwendet werden -> entferne diese namespace deklaration und wrappe die jeweilige chunks in einen namespace {} block
  • wenn in der datei ein oder mehrere namespace {} blöcke verwendet werden -> nimm die datei so wie sie ist

ich weiß nicht, ob zur zeit die dateien "zusammen geregext werden", aber es gibt da verschiedene parser, um php dateien sauber zu parsen und dann die ASTs zusammen zu fügen und zu dumpen.

@leofeyer
Copy link
Member

Behoben in bf11970.

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

No branches or pull requests

6 participants