Autor Thema: my $foo = $bar if $baz; # UNDEFINED!  (Gelesen 3530 mal)

Offline jw9

  • New Member
  • *
  • Beiträge: 37
my $foo = $bar if $baz; # UNDEFINED!
« am: 20 Januar 2021, 12:04:16 »
Hallo allerseits,

bin gerade dabei, 00_TUL.pm zu debuggen und habe dabei festgestellt, dass das im Betreff genannte Konstrukt

  my $foo = $bar if $baz;

kreuz und quer von sehr vielen Modulen verwendet wird.

Dieses Konstrukt ist OFFIZIELL nicht zulässig. Ich zitiere aus "perldoc perlsyn":
Zitat
NOTE: The behaviour of a "my", "state", or "our" modified with a statement
modifier conditional or loop construct (for example, "my $x if ...") is
undefined. The value of the "my" variable may be "undef", any previously
assigned value, or possibly anything else. Don't rely on it. Future
versions of perl might do something different from the version of perl you
try it out on. Here be dragons.

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #1 am: 24 Januar 2021, 21:57:01 »
In diesen Modulen ist das fehlerhafte Konstrukt enthalten:

$ find -name '*.p[lm]' -print0 |xargs -0 perl -ne '$/=";"; print qq=$ARGV\n= if /my[^a-zA-Z_0-9].*?=.*?[^a-zA-Z0-9\$]if[^a-zA-Z0-9]/ ' |sort|uniq -c|sort -rn
     68 ./FHEM/00_SONOS.pm
     30 ./FHEM/93_DbRep.pm
     30 ./contrib/DS_Starter/93_DbRep.pm
     10 ./FHEM/22_HOMEMODE.pm
      8 ./FHEM/Meta.pm
      8 ./FHEM/10_MQTT_GENERIC_BRIDGE.pm
      8 ./FHEM/10_EnOcean.pm
      7 ./FHEM/93_DbLog.pm
      7 ./FHEM/39_Talk2Fhem.pm
      7 ./contrib/DS_Starter/93_DbLog.pm
      6 ./FHEM/51_Netzer.pm
      6 ./FHEM/21_SONOSPLAYER.pm
      6 ./FHEM/10_ZWave.pm
      5 ./FHEM/Unit.pm
      5 ./FHEM/98_PID20.pm
      5 ./FHEM/38_netatmo.pm
      5 ./contrib/HMCCU/FHEM/88_HMCCU.pm
      4 ./FHEM/88_HMCCU.pm
      3 ./FHEM/98_monitoring.pm
      3 ./FHEM/95_YAAHM.pm
      3 ./FHEM/95_FLOORPLAN.pm
      3 ./FHEM/88_xs1Bridge.pm
      3 ./FHEM/77_SMAEM.pm
      3 ./FHEM/74_Unifi.pm
      3 ./FHEM/72_XiaomiDevice.pm
      3 ./FHEM/72_FRITZBOX.pm
      3 ./FHEM/59_Weather.pm
      3 ./FHEM/59_LuftdatenInfo.pm
      3 ./FHEM/57_Calendar.pm
      3 ./FHEM/53_GHoma.pm
      3 ./FHEM/52_I2C_PCA9685.pm
      3 ./FHEM/52_I2C_DS1307.pm
      3 ./FHEM/37_Spotify.pm
      3 ./FHEM/34_SWAP.pm
      3 ./FHEM/34_ESPEasy.pm
      3 ./FHEM/30_HUEBridge.pm
      3 ./FHEM/21_HEOSPlayer.pm
      3 ./FHEM/01_FHEMWEB.pm
      3 ./FHEM/00_TUL.pm
      3 ./contrib/DS_Starter/77_SMAEM.pm
      3 ./contrib/betateilchen/57_Calendar.pm
      3 ./contrib/98_Sprinkle.pm
      3 ./contrib/98_FileLogConvert.pm
      2 ./FHEM/FritzBoxUtils.pm
      2 ./FHEM/98_telnet.pm
      2 ./FHEM/98_powerMap.pm
      2 ./FHEM/98_archetype.pm
      2 ./FHEM/96_allowed.pm
      2 ./FHEM/93_RFHEM.pm
      2 ./FHEM/90_SIGNALduino_un.pm
      2 ./FHEM/88_Itach_IRDevice.pm
      2 ./FHEM/76_msgDialog.pm
      2 ./FHEM/71_YAMAHA_NP.pm
      2 ./FHEM/70_VIERA.pm
      2 ./FHEM/55_DWD_OpenData.pm
      2 ./FHEM/52_I2C_PCA9532.pm
      2 ./FHEM/50_TelegramBot.pm
      2 ./FHEM/48_BlinkCamera.pm
      2 ./FHEM/10_EIB.pm
      2 ./FHEM/10_CUL_HM.pm
      2 ./FHEM/00_TCM.pm
      2 ./FHEM/00_Neuron.pm
      2 ./configDB.pm
      1 ./lib/FHEM/SynoModules/SMUtils.pm
      1 ./FHEM/OWX_FRM.pm
      1 ./FHEM/lib/ProtoThreads.pm
      1 ./FHEM/98_todoist.pm
      1 ./FHEM/98_serviced.pm
      1 ./FHEM/98_notice.pm
      1 ./FHEM/98_JsonList2.pm
      1 ./FHEM/98_inotify.pm
      1 ./FHEM/98_IF.pm
      1 ./FHEM/98_HMtemplate.pm
      1 ./FHEM/98_DOIFtools.pm
      1 ./FHEM/98_DOIF.pm
      1 ./FHEM/98_DLNARenderer.pm
      1 ./FHEM/98_autocreate.pm
      1 ./FHEM/92_FileLog.pm
      1 ./FHEM/89_ESPEInk.pm
      1 ./FHEM/74_UnifiVideo.pm
      1 ./FHEM/74_Nmap.pm
      1 ./FHEM/73_MPD.pm
      1 ./FHEM/60_allergy.pm
      1 ./FHEM/52_I2C_LCD.pm
      1 ./FHEM/52_I2C_HDC1008.pm
      1 ./FHEM/45_TRX.pm
      1 ./FHEM/44_ROLLO.pm
      1 ./FHEM/40_RFXCOM.pm
      1 ./FHEM/38_CO20.pm
      1 ./FHEM/38_Broadlink.pm
      1 ./FHEM/36_Shelly.pm
      1 ./FHEM/36_ShellyMonitor.pm
      1 ./FHEM/32_SYSSTAT.pm
      1 ./FHEM/32_speedtest.pm
      1 ./FHEM/30_LIGHTIFY.pm
      1 ./FHEM/26_KM273.pm
      1 ./FHEM/12_HMS.pm
      1 ./FHEM/10_KNX.pm
      1 ./FHEM/10_Itach_IR.pm
      1 ./FHEM/10_FRM.pm
      1 ./FHEM/02_FTUISRV.pm
      1 ./FHEM/00_ZWDongle.pm
      1 ./FHEM/00_SIGNALduino.pm
      1 ./FHEM/00_HMUARTLGW.pm
      1 ./FHEM/00_CUL.pm
      1 ./contrib/YAF/FHEM/YAF/libs/json/JSON/backportPP.pm
      1 ./contrib/betateilchen/51_BBB_BMP180.pm
      1 ./contrib/98_dev_proxy.pm
      1 ./contrib/31_PLAYBULB.pm

Da mag vielleicht der eine oder andere false-positive dabei sein. Bei oberflächlichen überfliegen habe ich aber keine gesehen.

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #2 am: 12 Februar 2021, 10:27:47 »
Gibt es denn keine Meinungen hierzu?

Online OdfFhem

  • Sr. Member
  • ****
  • Beiträge: 789
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #3 am: 12 Februar 2021, 11:27:28 »
@jw9

Vermutlich ist dieser Beitrag nicht in der richtigen Rubrik gelandet.
Er passt vielleicht eher in "FHEM Forum --- FHEM - Entwicklung --- FHEM Development"
oder "FHEM Forum --- FHEM - Entwicklung --- FHEM Development --- Perl Ecke".

Desweiteren würde ein verständlicherer Betreff mehr Aufmerksamkeit erzeugen.

Ich hatte mir Deine Liste mal angeschaut und zufällig zwei selbstgenutzte Dateien ausgewählt:
- 02_FTUISRV.pm enthält keine "schädliche" Zeile
- 92_FileLog.pm enthält genau eine "schädliche" Zeile.

Vielleicht sollte man an so einem zentralen Modul mal aufzeigen, was "falsch" bzw. "nicht ganz richtig" ist und was man stattdessen tun sollte/müsste ...

Fraglich ist aber, ob es da tatsächlich zu einem Problem kommen kann oder kommen wird. Weit verbreitet ist zwar die Meinung
Zitat
... its not allowed and can result in undesired behaviour (e.g. crashing). Well, it won't crash, but it could :) ...
, aber bis jetzt hat man ja scheinbar Glück gehabt ...

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 24516
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #4 am: 12 Februar 2021, 11:34:50 »
Zitat
Gibt es denn keine Meinungen hierzu?
Doch: vielen Dank fuer die Arbeit, das Aendern meiner Module ist auf meinem TODO, ist aber leider etwas nach unten gerutscht.
Wenn wir eine sichere Methode fuers Pruefen haetten, wuerde ich das auch in unseren SVN-Hook einbauen.

Dieses Thema gehoert nach "FHEM-Development" (einfach Zugang beantragen), Wunschliste lese ich (wie vmtl. etliche Developer auch) nur zufaellig. Keine Ahnung, wer diesen Forumsbschnitt angelegt hat, und wozu.

Zitat
, aber bis jetzt hat man ja scheinbar Glück gehabt ...
Mich hat es schon gebissen. Die Probleme sind nicht offensichtlich: Fehlermeldungen gibt es keine, nur die Ergebnisse sind je nach input falsch.

Offline Beta-User

  • Developer
  • Hero Member
  • ****
  • Beiträge: 15802
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #5 am: 12 Februar 2021, 11:48:56 »
Auch von meiner Seite: Danke für den Schubser.

Betr. MQTT_GENERIC_BRIDGE siehe https://svn.fhem.de/trac/changeset/23652/
(Falls du beim grep noch was findest: könnte sein, dass es die eine oder andere auskommentierte Zeile gibt, auf die das (noch) matcht).
Server: HP-T620@Debian 10, aktuelles FHEM + ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn:MySensors, Weekday-&RandomTimer, Twilight,  AttrTemplate {u.a. mqtt2, mysensors, zwave}

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1775
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #6 am: 12 Februar 2021, 12:24:43 »
Gibt es denn keine Meinungen hierzu?

Ich hab mal ein paar Issues auf Github aufgemacht ...
Gefällt mir Gefällt mir x 1 Liste anzeigen

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #7 am: 12 Februar 2021, 12:40:59 »
Vermutlich ist dieser Beitrag nicht in der richtigen Rubrik gelandet.

Er passt vielleicht eher in "FHEM Forum --- FHEM - Entwicklung --- FHEM Development"
oder "FHEM Forum --- FHEM - Entwicklung --- FHEM Development --- Perl Ecke".
Auf "Development" hatte ich keine Schreibrechte und die Hürden welche zu erhalten erschienen mir doch sehr hoch...

Zitat
Desweiteren würde ein verständlicherer Betreff mehr Aufmerksamkeit erzeugen.
Hmmm... Ja... Ich wollte halt möglichst präzise sein...

Zitat
Ich hatte mir Deine Liste mal angeschaut und zufällig zwei selbstgenutzte Dateien ausgewählt:
- 02_FTUISRV.pm enthält keine "schädliche" Zeile
Ja, dass da false-positives sein können hatte ich geschrieben. einen kompletten perl-Parser in einem regex unterzubringen ist nicht ganz einfach  ;) In diesem Fall war die Sequenz "-if\s" innerhalb des regex der Auslöser für den false-positive.

Ich schätze mal, dass von den 452 Hits wohl 5% bis 10% false-positives sein könnten.

Zitat
- 92_FileLog.pm enthält genau eine "schädliche" Zeile.
Und genau um solche Zeilen geht es ja...

Zitat
Vielleicht sollte man an so einem zentralen Modul mal aufzeigen, was "falsch" bzw. "nicht ganz richtig" ist und was man stattdessen tun sollte/müsste ...
Naja, das hatte ich ja schon im ersten Beitrag dieses Threads aus der Doku zitiert...

Zitat
Fraglich ist aber, ob es da tatsächlich zu einem Problem kommen kann oder kommen wird. Weit verbreitet ist zwar die Meinung, aber bis jetzt hat man ja scheinbar Glück gehabt ...
Das Problem ist, dass der Wert der Variablen nicht vorhersagbar ist, wenn die Bedingung unwahr ist. Wenn dann mit "zufälligen" Werten weitergearbeitet wird, kann ales mögliche rauskommen. Das Verhalten kann sich dann auch mit der perl-Version verändern.

Im Übrigen trifft das nicht nur für "if" zu sondern auch auf "unless" und loop-Konstrukte. Wobei loop-Konstrukte wohl kaum jemand im Zusammenhang mit einer Variablen-Deklaration verwenden würde.

Hier nochmal der Einzeiler erweitert, so dass er auch auf "unless" matcht und die verdächtige Stelle ausgibt:
find /opt/fhem -name '*.p[lm]' -print0 |xargs -0 perl -ne '$/=";"; s/^\n//m;print qq=$ARGV:\n$_\n---\n= if /my[^a-zA-Z_0-9].*?=.*?[^a-zA-Z0-9\$](if|unless)[^a-zA-Z0-9]/ '|less

Oh, und dabei stolpert man auch über Highlights wie
  if(defined ($streams)){
    my @streams=@{$streams} unless not defined ($streams);
  [ ... ]
Wobei dieser Ausschnitt unbedenklich ist im Sinne dieses Threads. Aber doppelte Verneinung und doppelte Abfrage sind auch nicht gerade zuträglich für Verständlichkeit.

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #8 am: 12 Februar 2021, 12:48:45 »
Wenn wir eine sichere Methode fuers Pruefen haetten, wuerde ich das auch in unseren SVN-Hook einbauen.
Ich denke nicht dass es mit halbwegs vertretbarem Aufwand möglich ist eine Methode zu entwerfen die sicher genug für einen Hook wäre.

Die einzige wirklich sichere Methode wäre, wenn der perl-Interpreter selbst warnings werfen würde.

Soweit ich weiss gibt es dafür jedoch keine Möglichkeit. Eigentlich schade, wenn man bedenkt, dass es ein sehr weit verbreitetes Konstrukt ist.

Offline Beta-User

  • Developer
  • Hero Member
  • ****
  • Beiträge: 15802
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #9 am: 12 Februar 2021, 12:56:30 »
Auf "Development" hatte ich keine Schreibrechte und die Hürden welche zu erhalten erschienen mir doch sehr hoch...
? Wäre eine einfache Anfrage, und es würde ggf. auch ausreichen, wenn du dich als "Tester" melden wolltest, falls dir der (in der Sache wenig aussagefähige) Titel "Developer" nicht gefällt...
Server: HP-T620@Debian 10, aktuelles FHEM + ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn:MySensors, Weekday-&RandomTimer, Twilight,  AttrTemplate {u.a. mqtt2, mysensors, zwave}

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4534
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #10 am: 12 Februar 2021, 17:16:51 »
Das Thema hatte IMHO doch auch RichardCZ letztes Jahr an den Pranger gestellt und ich dachte es überall erschlagen zu haben , aber
1 ./FHEM/73_MPD.pmTHX4Info, wird demnächst erledigt.

Edit : Developer wäre wirklich der bessere Platz, habe es eben auch nur durch Zufall gefunden ....
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 24516
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #11 am: 12 Februar 2021, 21:32:18 »
Ich habe meine Module geaendert, alles der Sorte "my $xx = ... if(...)" umgebaut.
Soweit ich sehe, habe ich durch die Aenderungen nirgendwo ein Problem gefixt.

00_CUL.pm
00_ZWDongle.pm
01_FHEMWEB.pm
10_ZWave.pm
12_HMS.pm
92_FileLog.pm
96_allowed.pm
98_JsonList2.pm
98_autocreate.pm
98_telnet.pm
FritzBoxUtils.pm

Offline Sidey

  • Developer
  • Hero Member
  • ****
  • Beiträge: 2594
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #12 am: 12 Februar 2021, 21:48:55 »
Zunächst mal danke für den Hinweis. Hatte ich so exakt nicht auf dem Radar. :)
Grundsätzlich leuchtet es mir aber ein, dass eine Variable unter Umständen nicht existiert wenn das in einem IF Block läuft.
Im PBP stand da auch was zu im Kontext von Schleifen etc. :)

An einem automatisierten Test hatte ich auch gedacht, würde aber erwarten dass perlcritic das bereits detektieren kann.

Die Regex von dir, fängt meiner Ansicht nach folgenden false positive fall:

my %gets = (  # NameOFCommand =>  StyleMod for Fhemweb, SubToCall if get is executed, String to send to uC, sub called with response, regex to verify response,
Grüße Sidey

Signalduino, Homematic, Raspberry Pi, Mysensors, MQTT, Alexa

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4534
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #13 am: 13 Februar 2021, 08:41:44 »
würde aber erwarten dass perlcritic das bereits detektieren kann.
ja das ist mir heute morgen auch erst eingefallen, bei einem ersten Test meckert percritic sogar mit Level 5:
Variable declared in conditional statement at line 847, column 5.  Declare variables outside of the condition.  (Severity: 5)
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #14 am: 13 Februar 2021, 13:19:04 »
Grundsätzlich leuchtet es mir aber ein, dass eine Variable unter Umständen nicht existiert wenn das in einem IF Block läuft.
Im PBP stand da auch was zu im Kontext von Schleifen etc. :)
Grundsätzlich würde das Problem auch bei Schleifen existieren. In freier Wildbahn halte ich das aber für eher unwahrscheinlich. Wer würde schon auf die Idee kommen etwas wie "my $foo = 'bar' while $baz;" zu schreiben?

Und tatsächlich: wenn ich den regex um "while" und "until" erweitere, habe ich 13 weitere matches, allesamt false-positives.

Zitat
An einem automatisierten Test hatte ich auch gedacht, würde aber erwarten dass perlcritic das bereits detektieren kann.
Bei perlcritic habe ich sehr gemischte Gefühle. Ich habe mal probehalber einige meiner Projekte durch perlcritic gejagt und Tonnen von Warnings erhalten, die allesamt harmlos/unbegründet waren. Ein Tool das 99% false-positives liefert ist keine echte Hilfe, zumindest nicht wenn man das automatisieren will... Ich muss aber zugeben, dass ich perlcritic in der default-configuration verwendet habe. Vermutlich kann man das so konfigurieren, dass es sinnvollere Ergebnisse liefert.

Zitat
Die Regex von dir, fängt meiner Ansicht nach folgenden false positive fall:

my %gets = (  # NameOFCommand =>  StyleMod for Fhemweb, SubToCall if get is executed, String to send to uC, sub called with response, regex to verify response,
Wie gesagt: das ist eine einfache regex und kein vollständiger perl-parser. Eine regex ist grundsätzlich nicht in der Lage sauber zu arbeiten, sobald Klammern und/oder quotes im Spiel sind. Dazu benötigt es einen Stack, den eine regex nuneinmal nicht hat. Und nein, den Kommentar einfach wegclipsen geht nicht, denn der könnte gequotet sein und man geht das Risiko ein, ein true-positive zu verfehlen.

Solange der Anteil an false-positives kleiner als (sagenwirmal) 30% ist, ist es wesentlich effizienter die true-positives abzuarbeiten als das matching-Verfahren zu optimieren. YMMV!

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1775
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #15 am: 13 Februar 2021, 14:54:47 »
Bei perlcritic habe ich sehr gemischte Gefühle. Ich habe mal probehalber einige meiner Projekte durch perlcritic gejagt und Tonnen von Warnings erhalten, die allesamt harmlos/unbegründet waren. Ein Tool das 99% false-positives liefert ist keine echte Hilfe, zumindest nicht wenn man das automatisieren will... Ich muss aber zugeben, dass ich perlcritic in der default-configuration verwendet habe. Vermutlich kann man das so konfigurieren, dass es sinnvollere Ergebnisse liefert.

Genau. Man kann z.B. nur einen einzelnen Test aufrufen, in diesem Falle wäre das ProhibitConditionalDeclarations.

Ich habe übrigens mit P::C die Erfahrung gemacht, dass es genau 0 false-positives geliefert hat. Dazu habe ich mir jeden Fall dann auch im Buch angeschaut und versucht nachzuvollziehen, warum P::C meckert.
« Letzte Änderung: 13 Februar 2021, 14:56:26 von Christoph Morrison »

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1775
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #16 am: 13 Februar 2021, 16:23:00 »
Das hier ist die Liste, die mir P::C ausspuckt:

./FHEM/10_MQTT_GENERIC_BRIDGE.pm
./FHEM/98_notice.pm
./FHEM/21_SONOSPLAYER.pm
./FHEM/26_KM273.pm
./FHEM/60_allergy.pm
./FHEM/98_Modbus.pm
./FHEM/98_SVG.pm
./FHEM/98_inotify.pm
./FHEM/74_UnifiVideo.pm
./FHEM/90_SIGNALduino_un.pm
./FHEM/14_SD_UT.pm
./FHEM/52_I2C_LCD.pm
./FHEM/76_SMAPortal.pm
./FHEM/50_TelegramBot.pm
./FHEM/10_FRM.pm
./FHEM/52_I2C_PCA9532.pm
./FHEM/38_netatmo.pm
./FHEM/00_HMUARTLGW.pm
./FHEM/74_AMADDevice.pm
./FHEM/77_UWZ.pm
./FHEM/RESIDENTStk.pm
./FHEM/95_YAAHM.pm
./FHEM/14_SD_BELL.pm
./FHEM/74_Nmap.pm
./FHEM/10_EIB.pm
./FHEM/98_DOIFtools.pm
./FHEM/98_telnet.pm
./FHEM/48_BlinkCamera.pm
./FHEM/22_HOMEMODE.pm
./FHEM/20_FRM_SERVO.pm
./FHEM/21_HEOSGroup.pm
./FHEM/00_TCM.pm
./FHEM/53_GHoma.pm
./FHEM/92_FileLog.pm
./FHEM/10_KNX.pm
./FHEM/95_FLOORPLAN.pm
./FHEM/00_TUL.pm
./FHEM/88_HMCCU.pm
./FHEM/98_monitoring.pm
./FHEM/93_DbRep.pm
./FHEM/21_HEOSPlayer.pm
./FHEM/20_FRM_PWM.pm
./FHEM/21_HEOSMaster.pm
./FHEM/01_FHEMWEB.pm
./FHEM/10_EnOcean.pm
./FHEM/98_PID20.pm
./FHEM/77_SMAEM.pm
./FHEM/75_msgConfig.pm
./FHEM/98_template.pm
./FHEM/98_JsonList2.pm
./FHEM/73_GardenaSmartBridge.pm
./FHEM/88_Itach_IRDevice.pm
./FHEM/59_LuftdatenInfo.pm
./FHEM/55_DWD_OpenData.pm
./FHEM/57_Calendar.pm
./FHEM/00_SONOS.pm
./FHEM/88_xs1Bridge.pm
./FHEM/93_DbLog.pm
./FHEM/00_CUL.pm
./FHEM/Unit.pm
./FHEM/98_archetype.pm
./FHEM/98_DLNARenderer.pm
./FHEM/76_msgDialog.pm
./FHEM/39_Talk2Fhem.pm
./FHEM/38_CO20.pm
./FHEM/51_Netzer.pm
./FHEM/30_HUEBridge.pm
./FHEM/44_ROLLO.pm
./FHEM/lib/ProtoThreads.pm
./FHEM/10_ZWave.pm
./FHEM/98_structure.pm
./FHEM/98_serviced.pm
./FHEM/32_speedtest.pm
./FHEM/70_VIERA.pm
./FHEM/FritzBoxUtils.pm
./FHEM/98_todoist.pm
./FHEM/12_HMS.pm
./FHEM/74_Unifi.pm
./FHEM/32_SYSSTAT.pm
./FHEM/10_Itach_IR.pm
./FHEM/40_RFXCOM.pm
./FHEM/34_ESPEasy.pm
./FHEM/72_XiaomiDevice.pm
./FHEM/96_allowed.pm
./FHEM/71_YAMAHA_NP.pm
./FHEM/10_CUL_HM.pm
./FHEM/34_SWAP.pm
./FHEM/98_powerMap.pm
./FHEM/50_HP1000.pm
./FHEM/00_Neuron.pm
./FHEM/OWX_FRM.pm
./FHEM/93_RFHEM.pm
./FHEM/96_Snapcast.pm
./FHEM/98_HMtemplate.pm
./FHEM/59_Twilight.pm
./FHEM/98_autocreate.pm
./FHEM/52_I2C_HDC1008.pm
./FHEM/50_MOBILEALERTSGW.pm
./FHEM/38_Broadlink.pm
./FHEM/00_ZWDongle.pm
./FHEM/30_LIGHTIFY.pm
./FHEM/10_RESIDENTS.pm
./FHEM/37_Spotify.pm
./FHEM/72_FRITZBOX.pm
./FHEM/52_I2C_PCA9685.pm
./FHEM/Meta.pm
./FHEM/45_TRX.pm
./FHEM/52_I2C_DS1307.pm
./FHEM/59_Weather.pm
./contrib/betateilchen/51_BBB_BMP180.pm
./contrib/betateilchen/57_Calendar.pm
./contrib/98_dev_proxy.pm
./contrib/31_PLAYBULB.pm
./contrib/98_FileLogConvert.pm
./contrib/YAF/FHEM/YAF/libs/json/JSON/backportPP.pm
./contrib/98_Sprinkle.pm
./contrib/70_ONKYO_AVR_PULL.pm
./contrib/HMCCU/FHEM/88_HMCCU.pm
./contrib/DS_Starter/76_SMAPortal.pm
./contrib/DS_Starter/93_DbRep.pm
./contrib/DS_Starter/77_SMAEM.pm
./contrib/DS_Starter/93_DbLog.pm
./contrib/86_FS10.pm
./configDB.pm
./fhem.pl

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #17 am: 13 Februar 2021, 20:19:04 »
Soweit ich sehe, habe ich durch die Aenderungen nirgendwo ein Problem gefixt.
Echt? Dann hat es ja nicht geschadet...

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #18 am: 14 Februar 2021, 01:17:38 »
Ich habe übrigens mit P::C die Erfahrung gemacht, dass es genau 0 false-positives geliefert hat. Dazu habe ich mir jeden Fall dann auch im Buch angeschaut und versucht nachzuvollziehen, warum P::C meckert.

Ich habe vielmehr das Gefühl dass perlcritic in erster Linie den Vertrieb von PBP ankurbeln soll. Diese Annahme liegt aufgrund der Ausgaben von perlcritic sehr nahe!

Hier die Ausgabe für eines meiner Projekte, diese "Fehlermeldungen" will ich weiter unten näher beleuchten:

$ perlcritic  /m/s/rep/git/public/jw/netarchive/narun
"require" statement with library name as string at line 201, column 12.  Use a bareword instead.  (Severity: 5)
"require" statement with library name as string at line 202, column 12.  Use a bareword instead.  (Severity: 5)
"require" statement with library name as string at line 203, column 12.  Use a bareword instead.  (Severity: 5)
"return" statement with explicit "undef" at line 843, column 5.  See page 199 of PBP.  (Severity: 5)
Don't modify $_ in list functions at line 1761, column 5.  See page 114 of PBP.  (Severity: 5)
Don't modify $_ in list functions at line 2834, column 5.  See page 114 of PBP.  (Severity: 5)
"require" statement with library name as string at line 3065, column 12.  Use a bareword instead.  (Severity: 5)
"return" statement with explicit "undef" at line 3283, column 5.  See page 199 of PBP.  (Severity: 5)

50% der "Fehler" beziehen sich auf "require". Also schauen wir uns diese als erstes an:

# Read config
0 ||
    eval { require "/etc/na/$conf/na.conf" } ||
    eval { require "/etc/na/$conf.conf" }    ||
    eval { require "/etc/na.conf" }          ||
    die "Can't read config '$conf'";

Hier wird require verwendet, um Konfigurationsdateien zu lesen/parsen. Wie bitte soll ich hier ein bareword verwenden? Vorschläge, das besser zu formulieren nehme ich gerne entgegen...

    eval { require "sys/resource.ph"; $prio_process = &PRIO_PROCESS; };

Auch hier würde mich interessieren, wie das als "Bareword" formuliert werden soll... Zwar könnte man ein "foo/bar.pm" als foo::bar formulieren. Aber hier ist es eben kein .pm sondern ein .ph


Weitere 25% der "Fehler"  werden in Zeilen 5 und 6 moniert:

   my @files = @$files;
   grep {s/\n//g; s/$/\n/; } @files; # ensure every entry has exactly one NL

sowie

    my @client_cmd = @{$info->{"command"}};
    grep { s/__CLIENT__/$client/g; } @client_cmd;

Da Seite 114 von PBP bei "Blick-ins-Buch" verfügbar ist, kann ich hier nachvollziehen was perlcritic hier anmoniert. Und es ist klar, dass perlcritic den entsprechende Absatz in PBP vollkommen fehlerhaft umsetzt.

In beiden Fällen wird EXPLIZIT eine Kopie gemacht, auf der grep operiert. Im Gegensatz zur Beschreibung in PBP wird die ursprüngliche Datenstruktur nicht modifiziert.

Spätestens an dem Umstand, dass der Rückgabewert von grep nicht verwendet wird, hätte für perlcritic der Hinweis sein müssen, dass der einzige Zweck dieses Konstrukts der Seiteneffekt ist die Eingabestruktur zu modifizieren.

Insofern haben sowohl perlcritic als auch PBP unrecht: die Modifikation des Arrays ist hier beabsichtigt und der einzige Zweck, dass grep hier überhaupt verwendet wird.

Und da der Rückgabewert von grep nirgendwo zugewiesen wird, ist glasklar, dass der Seiteneffekt (eben diese Modifikation des Eingabearrays) in diesem Fall beabsichtigt ist. Andernfalls wäre dieser Code wirkungslos und könnte ersatzlos gestrichen werden.

Kommen wir zu den verbleibenden 25%

Hier wird moniert, dass "return" ein "undef" liefert. HIer tue ich mir schwer, diese Argumentation nachzuvollziehen. IMHO kann es in bestimmten Situationen durchaus sinnvoll sein, im Fehlerfall "undef" zu liefern um eine klare Unterscheidung zu gültigen Rückgabewerten zu haben. Leider sagt perlcritic nicht, was daran so schlimm sein soll. Vielmehr soll ich ein Buch kaufen, um zu erfahren was das soll. Nein danke! Leider ist Seite 199 von PBP bei "Blick-ins-Buch" nicht freigeschaltet. Vielleicht kannst Du dazu einige klärende Worte beitragen?

Unterm Strich sind 75% der Meldungen von perlcritic schlicht unsinnig. Bei den verbleibenden 25% sehe ich ebenfalls nicht, was perlcritic daran zu kritisieren hat. Nur eines sehe ich glasklar: perlcritic nötigt den Anwender, PBP zu kaufen. Ich glaube ja gerne, dass das ein gutes Buch ist. Aber was perlcritic daraus macht ist noch nicht einmal ansatzweise überzeugend....

Gib mal bitte Butter bei die Fische: Was steht in PBP auf Seite 199, damit ich wenigstens zwei der acht von perlcritic gemeldeten Fehlermeldungen ernst nehmen kann...

Insgesamt verbleiben bei mir die gemischten Gefühle bei perlcritic: only noise, no signal!

perlcritic mag sinnvoll sein für jemanden der mal in perl hineinschnuppert. Darüberhinaus kann ich aber keinen echten Nutzwert erkennen.

Wie immer: YMMV!


Offline Beta-User

  • Developer
  • Hero Member
  • ****
  • Beiträge: 15802
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #19 am: 14 Februar 2021, 07:17:05 »
perlcritic.com kennst du?
Da kannst du erfahren, warum ein "return;" statt eines "return undef;" empfohlen wird (samt Erklärungen zum Rest).Wenn man sicher ist (!), dass die critic unberechtigt ist:  “##no critics qw(XYZ)" als Pflaster hin und jeder kann sehen, dass der code an der Stelle entsprechend gecheckt worden ist...
Server: HP-T620@Debian 10, aktuelles FHEM + ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn:MySensors, Weekday-&RandomTimer, Twilight,  AttrTemplate {u.a. mqtt2, mysensors, zwave}

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 4534
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #20 am: 14 Februar 2021, 07:25:23 »
zum Thema return undef hatte Richard einiges geschrieben -> https://forum.fhem.de/index.php/topic,109592.msg1035806/topicseen.html#msg1035806

@jw9 , schick mir mal ne PM mit einer gültigen Email Adresse von dir dann kann man auch das Seite 199 und ff Prob lösen :)
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 24516
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #21 am: 14 Februar 2021, 10:22:25 »
Zitat
Zitat
Soweit ich sehe, habe ich durch die Aenderungen nirgendwo ein Problem gefixt.
Echt? Dann hat es ja nicht geschadet...
Das ist eine Verdrehung meiner Aussage.
Ich habe nur gesagt, dass mein "Fix" kein FHEM-Bug behoben hat. Dass ich dadurch keinen erzeugt habe, dass hoffe ich zwar, bewiesen ist das aber noch nicht.
Jedenfalls sind die vorherigen Aenderungen in "svn blame" jetzt umstaendlicher zu finden, aber was tut man nicht alles, um von automatischen Warnprogrammen seine Ruhe zu haben.

Offline Sidey

  • Developer
  • Hero Member
  • ****
  • Beiträge: 2594
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #22 am: 14 Februar 2021, 11:22:35 »
Als ich perlcritic ins Spiel brachte, hatte ich nicht geahnt, dass wieder die Diskussion ausbricht, ob best practices eine gute oder schlechte Sache sind.

Mir ging es primär um den im Betreff dargestellten "Fehler" und dass die regex auch Stellen findet, die kein Problem sind, weil es ein Kommentar ist.
Ich bin mir sicher, dass die regex optimiert werden könnte um diese false positives nicht zu finden.
Das Rad wurde aber schon erfunden und es ist rund. So bitte ich den Hinweis auf perlcritic zu verstehen.

Da perlcritic von Menschen gemacht wurde und Menschen Fehler machen, bitte keine unfehlbarkeit erwarten.

Wenn ich das Ursprüngliche Thema aufgreife, dass Variablen außerhalb von Bedingungen definiert werden sollten, dann lässt sich das vorkommen, ohne Perlcritic konfigurieren zu müssen mit folgendem Befehl auffinden:

perlcritic app/FHEM/ app/fhem.pl app/t/ -5 | grep condition

Da perlcritic noch läuft und mein Zeilenpuffer (200 Zeilen) ohnehin nicht ausreicht um es hier zu posten, sollte das jeder selbst für sich oder seinen Code machen.

Hier ein klitzekleiner Auszug der letzten Vorkommnisse und vorweg, hier werden auch Bedingungen gefunden die nicht durch das if statement ausgelöst werden.


app/FHEM/00_TCM.pm: Variable declared in conditional statement at line 525, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/00_TCM.pm: Variable declared in conditional statement at line 1202, column 13.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/72_XiaomiDevice.pm: Variable declared in conditional statement at line 340, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/72_XiaomiDevice.pm: Variable declared in conditional statement at line 3538, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/72_XiaomiDevice.pm: Variable declared in conditional statement at line 3539, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/88_Itach_IRDevice.pm: Variable declared in conditional statement at line 87, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/88_Itach_IRDevice.pm: Variable declared in conditional statement at line 158, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 862, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 7367, column 5.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14560, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14561, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14562, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14565, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14567, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 14570, column 9.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 15312, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 15372, column 5.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/10_EnOcean.pm: Variable declared in conditional statement at line 15402, column 5.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 1547, column 5.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 1609, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2632, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2714, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2718, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2745, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2798, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 2974, column 5.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 3012, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/22_HOMEMODE.pm: Variable declared in conditional statement at line 3013, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/98_monitoring.pm: Variable declared in conditional statement at line 111, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/98_monitoring.pm: Variable declared in conditional statement at line 190, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/98_monitoring.pm: Variable declared in conditional statement at line 420, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/32_SYSSTAT.pm: Variable declared in conditional statement at line 71, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/88_xs1Bridge.pm: Variable declared in conditional statement at line 122, column 2.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/88_xs1Bridge.pm: Variable declared in conditional statement at line 592, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/88_xs1Bridge.pm: Variable declared in conditional statement at line 653, column 2.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/60_allergy.pm: Variable declared in conditional statement at line 140, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/34_ESPEasy.pm: Variable declared in conditional statement at line 531, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/34_ESPEasy.pm: Variable declared in conditional statement at line 532, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/34_ESPEasy.pm: Variable declared in conditional statement at line 2742, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/59_LuftdatenInfo.pm: Variable declared in conditional statement at line 91, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/59_LuftdatenInfo.pm: Variable declared in conditional statement at line 184, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/59_LuftdatenInfo.pm: Variable declared in conditional statement at line 212, column 3.  Declare variables outside of the condition.  (Severity: 5)
app/FHEM/lib/ProtoThreads.pm: Variable declared in conditional statement at line 132, column 7.  Declare variables outside of the condition.  (Severity: 5)
app/fhem.pl: Variable declared in conditional statement at line 1470, column 5.  Declare variables outside of the condition.  (Severity: 5)



Und wer sich jetzt fragt, wie er das für seinen Code im Blick halten könnte, ohne dass gleich ein SVN Hook das einchecken verhindert, der könnte sich hier eine Ideen holen.

Grüße Sidey
Signalduino, Homematic, Raspberry Pi, Mysensors, MQTT, Alexa
Zustimmung Zustimmung x 1 Liste anzeigen

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #23 am: 14 Februar 2021, 11:58:34 »
perlcritic.com kennst du?
Danke für den Hinweis!

Jetzt weiss ich auch dass perlcritic in meinem Fall 100% false-positives geliefert hat. Perlcritic vergisst zu prüfen, ob die betreffende Funktion überhaupt irgendwann in einem list-context aufgerufen wird.

Zitat
Wenn man sicher ist (!), dass die critic unberechtigt ist:  “##no critics qw(XYZ)" als Pflaster hin und jeder kann sehen, dass der code an der Stelle entsprechend gecheckt worden ist...
Soweit kommt es noch, dass ich die Lesbarkeit des Codes opfere, um ein Tool ruhigzustellen das mir bislang 100% false-positives geliefert hat...

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1775
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #24 am: 14 Februar 2021, 13:19:43 »
perlcritic app/FHEM/ app/fhem.pl app/t/ -5 | grep condition

Oder einfach:
perlcritic --single-policy=ProhibitConditionalDeclarations

Offline jw9

  • New Member
  • *
  • Beiträge: 37
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #25 am: 14 Februar 2021, 13:38:03 »
Als ich perlcritic ins Spiel brachte, hatte ich nicht geahnt, dass wieder die Diskussion ausbricht, ob best practices eine gute oder schlechte Sache sind.
Sorry, aber hier hast Du mich komplett falsch verstanden!

Ich habe zu keinem Zeitpunkt irgendwelche Einwände gegen best practices vorgebracht! Ganz im Gegentei: ich halte eine gute Lesbarkeit, Verständlichkeit und vor allem auch Klarheit von Code für ein sehr wichtiges Qualitätskriterium. Da hätte so manch ein fhem-Modul durchaus Nachholbedarf.

Ich habe lediglich darauf hingewiesen, dass percritic es nicht schafft, die best practices auf die es selbst hinweist sauber zu erkennen.

Zitat
Mir ging es primär um den im Betreff dargestellten "Fehler" und dass die regex auch Stellen findet, die kein Problem sind, weil es ein Kommentar ist.
Dass da false-positives drin sind habe ich schon in dem Beitrag geschrieben, in dem diese regex zum ersten mal auftaucht. Das habe ich auch im weiteren Verlauf mehrfach wiederholt.

Zitat
Ich bin mir sicher, dass die regex optimiert werden könnte um diese false positives nicht zu finden.
Warum das keinen Sinn macht habe ich weiter oben auch schon geschrieben. Ich wiederhole das aber gerne nochmal:
  • ist eine regex nicht das richtige Mittel um hierarchische Strukturen zu parsen. Das war ein quick-n-dirty hack weil ich bei mehreren Modulen über solche Konstrukte gestolpert war und auf die schnelle sehen wollte ob das nur Zufall war oder häufiger vorkommt.
  • Solange der Anteil der False-positives unter 20% liegt, ist es sinnvoller die Zeit in das Fixen der true-positives zu stecken als die Erkennung zu optimieren.

Einen weiteren Grund möchte ich noch nachliefern:
  • perlcritic existiert (das kannte ich noch nicht als ich diesen thread eröffnete)

Wenn Du meinst man kann die regex entsprechend verbessern, möchte ich Dich nicht davon abhalten. Wäre eine gute Übung  ;). Vergiss dabei aber nicht, operatoren wie q, qq und qr zu berücksichtigen.

Hint: die perl-syntax ist enorm komplex. Jeder Versuch, das mit einer regex auch nur halbwegs sauber zu parsen ist von vornherein zum Scheitern verurteilt. Selbst emacs scheitert gelegentlich beim Syntax-highliting. Ich kenne sonst keine Aufgabe, bei der emacs scheitert ;D. Ein Highlight ist zB "m=>$foo", das von emacs gerne mal als Anfang einer regex gewertet wird. perl-code zu 100% korrekt parsen kann IMHO nur perl höchstselbst.

Zitat
Das Rad wurde aber schon erfunden und es ist rund. So bitte ich den Hinweis auf perlcritic zu verstehen.

ACK!

Und meinen Hinweis auf perlcritic möchte ich so verstanden wissen, dass das nicht der Weisheit letzter Schluss anzusehen ist.

Ich möchte niemandem ausreden perlcritic zu verwenden. Ganz im Gegenteil: wenn man perlcritic auf den fhem-tree loslässt, dann wirft es Tonnen an Warnings. Zu meinem Erstaunen muss ich bei den meisten davon perlcritic zustimmen:

root@kiste:/opt/fhem# find -name '*.p[lm]' -print0 | xargs -0 perlcritic -5 |wc -l
24082
root@kiste:/opt/fhem#

So langsam wird mir auch klar, warum perlcritic bei mir nur false-positives zutage fördert: die allermeisten Konstrukte die es anmoniert meide ich wie der Teufel das Weihwasser  8)

Zitat
Da perlcritic von Menschen gemacht wurde und Menschen Fehler machen, bitte keine unfehlbarkeit erwarten.
Das erwarte ich auch nicht. Aber oben war von einem commit-hook die Rede. Ich würde mich herzlich bedanken, wenn mir ein tool aufgrund von 100% false-positives den commit verweigert...

Offline Beta-User

  • Developer
  • Hero Member
  • ****
  • Beiträge: 15802
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #26 am: 14 Februar 2021, 13:55:52 »
Ich möchte niemandem ausreden perlcritic zu verwenden. Ganz im Gegenteil: wenn man perlcritic auf den fhem-tree loslässt, dann wirft es Tonnen an Warnings. Zu meinem Erstaunen muss ich bei den meisten davon perlcritic zustimmen:
Ebend.

Wer der Oberchecker ist, wird ggf. viele "Fehler" von vornherein nicht machen, mir als DAM hat es geholfen, potentiell (!) problematische Stellen zu finden und (weitestgehend) zu beseitigen, meistens sogar zugunsten der Lesbarkeit (die paar ##no critics - Stellen stören auch nicht weiter...), und dabei "nebenbei" das eine oder andere zu lernen.
(Ich habe nur Code von früheren Maintainern übernommen und nichts signifikantes wirklich selbst entwickelt; die "Ehemaligen" waren allesamt Leute, die mehr draufhaben/-hatten wie meinereiner, und ich hätte nicht gedacht, dass da so viel "critics"-Potential drin steckt...).
Aber klar: es ist ein Hilfsmittel, und wer es nicht braucht oder nicht mag oder aus bestimmten Gründen an der einen oder anderen Stelle eine andere Philosophie fährt: Es ist Perl, und da gab es doch das "there's more...".
Server: HP-T620@Debian 10, aktuelles FHEM + ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW, BT@OpenMQTTGw | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | RHASSPY
svn:MySensors, Weekday-&RandomTimer, Twilight,  AttrTemplate {u.a. mqtt2, mysensors, zwave}

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1775
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #27 am: 28 Februar 2021, 13:53:10 »
Ich möchte hier darauf hinweisen, dass mit Perl 5.30 ff. dieses Konstrukt einen fatal error produzieren wird. Flankierend dazu kommt, dass mit der nächsten Stable-Version von Debian (Bullseye) Perl 5.32 als Standard ausgeliefert werden wird, das umschließt dann früher oder später auch Raspbian und andere Debian-Derivate und damit auch einen größeren Kreis der FHEM-Userbasis.

Macht bitte entsprechende Änderungen in euren Modulen / im Core / in Libs.

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 24516
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #28 am: 01 März 2021, 09:08:04 »
Zitat
Ich möchte hier darauf hinweisen, dass mit Perl 5.30 ff. dieses Konstrukt einen fatal error produzieren wird.
Das kann ich nicht bestaetigen, jedenfalls gilt die Aussage nicht generell.
Ich hatte diese Konstrukte aus meinen Modulen entfernt, ich hatte aber vorher keine Warnungen, oder "fatal error".
Womoeglich lag es daran, dass in meinen Faellen dieser Konstrukt nicht der Schleife war. Ja, ich teste aktuell mit Perl 5.32.

Offline justme1968

  • Developer
  • Hero Member
  • ****
  • Beiträge: 20841
Antw:my $foo = $bar if $baz; # UNDEFINED!
« Antwort #29 am: 01 März 2021, 09:31:03 »
@jw9: danke. ich denke ich habe meine alle repariert.
FHEM5.4,DS1512+,2xCULv3,DS9490R,HMLAN,2xRasPi
CUL_HM:HM-LC-Bl1PBU-FM,HM-LC-Sw1PBU-FM,HM-SEC-MDIR,HM-SEC-RHS
HUEBridge,HUEDevice:LCT001,LLC001,LLC006,LWL001
OWDevice:DS1420,DS18B20,DS2406,DS2423
FS20:fs20as4,fs20bs,fs20di
AKCP:THS01,WS15
CUL_WS:S300TH