วิธีตรวจสอบฟิลด์ที่ประกาศแบบไดนามิกบนวัตถุที่มี codesniffer ใน PHP

หลังจากการปรับโครงสร้างใหม่ เรามีสิ่งนี้ในคลาสหนึ่งของเรา:

class FooBar
{
    // $foo was $bla before
    private $foo;

    public function setBlubbOnArrayOnlyOnce($value)
    {
        // $this->bla was forgotten during refactoring. Must be $this->foo
        if(!isset($this->bla['blubb'])) {
             $this->foo['blubb'] = $value;
        }
    }
}

ดังนั้นในท้ายที่สุด $this->foo['blubb'] จะถูกตั้งค่าไว้เสมอ ไม่ใช่เพียงครั้งเดียว สิ่งนี้เกิดขึ้นเนื่องจากวิธีการมหัศจรรย์ของ PHP เราไม่ต้องการให้สามารถเข้าถึงฟิลด์แบบไดนามิกได้ ดังนั้นฉันคิดว่าฉันแค่เพิ่มกฎ codesniffer แต่ฉันไม่พบเลยจึงถามฉันว่าทำไม

PHPStorm แสดงฟิลด์ที่ประกาศแบบไดนามิกโดยแจ้งให้ทราบที่นั่น แต่ฉันต้องการให้สิ่งนี้ล้มเหลวโดยอัตโนมัติด้วย codesniffer (หรือบางอย่างที่คล้ายกัน) ในระหว่างรอบการปรับใช้ของเรา

มีใครมีความคิดเกี่ยวกับเรื่องนี้หรือไม่? มีกฎเกณฑ์ที่ดีหรือไม่? ฉันควรเขียนของตัวเองหรือไม่ และอย่างไร? หรือมันจะเป็นแนวทางปฏิบัติที่ไม่ดีที่จะปิดการใช้งานมัน?

ข้อจำกัดความรับผิดชอบ: เราใช้การทดสอบ แต่บางครั้งคุณก็พลาดสิ่งต่าง ๆ... การป้องกันสิ่งนี้ตั้งแต่แรกจะเป็นการดี นอกจากนี้ โปรดอย่าคิดเขียนทับวิธีการวิเศษเลย ฉันไม่ต้องการที่จะมีลักษณะ/นามธรรมอะไรก็ตามในทุกชั้นเรียน


php getter-setter magic-methods codesniffer phpcodesniffer
person Kasihasi    schedule 13.08.2015    source แหล่งที่มา
comment
คุณสามารถค้นหาตัวแปรที่ไม่ได้กำหนดได้ไหม เนื่องจาก $this-›bla จะไม่ถูกประกาศ? คุณอาจต้องขยายโค้ดใน PHPCodeSniffer   -  person Steven Scott    schedule 13.08.2015
comment
ฉันกำลังพยายาม แต่ฉันหวังว่าจะมีวิธีที่ชัดเจนและง่ายในการทำ   -  person Kasihasi    schedule 14.08.2015
comment
คุณได้ถามใน Squizlabs (squizlabs.com) หรือ GitHub (github.com/squizlabs/PHP_CodeSniffer) เนื่องจาก Greg Sherwood ค่อนข้างตอบคำถามในอดีต   -  person Steven Scott    schedule 14.08.2015
comment
มันอาจจะไม่ง่ายขนาดนั้น โดยทั่วไปแล้ว $this->bla อาจถูกนิยามไว้ในคลาสพาเรนต์ Codesniffer ทำงานในระดับไฟล์/โทเค็น และหากคุณปฏิบัติตามมาตรฐานการเข้ารหัส PSR ก็จะไม่ทราบโครงสร้างคลาสพาเรนต์ (เนื่องจากอยู่ในไฟล์แยกต่างหาก)   -  person weirdan    schedule 08.01.2016
comment
@Weirdan นั่นอาจเป็นเรื่องจริง แต่คงจะดีถ้าสามารถตรวจพบคุณสมบัติที่ไม่ได้กำหนดได้ในคลาสธรรมดา โดยไม่ขยายคลาสอื่นใด   -  person maxhb    schedule 10.01.2016
comment
ทำไมคุณไม่ประกาศส่วนตัว $foo = []; เป็นอาร์เรย์ในคลาสเหรอ?   -  person Ravi    schedule 12.01.2016
comment
คุณได้ลองใช้ property_exists แล้วหรือยัง? php.net/manual/en/function.property-exists.php   -  person Ravi    schedule 12.01.2016

คำตอบ (3)


arrow_upward
2
arrow_downward

นี่ไม่ใช่ปัญหา codesniffer หรือ phpstorm และคุณไม่ต้องการแก้ไขปัญหานี้ด้วย codesniffer หรือ IDE IDE, codesniffer, phpdocumentor ฯลฯ -- นี่คือการวิเคราะห์แบบ "คงที่" และสำหรับการวิเคราะห์แบบไดนามิก คุณสามารถใช้เช่น phpunit.

หากคุณต้องการตรวจสอบการมีอยู่ของคุณสมบัติ คุณต้องใช้ฟังก์ชัน property_exists()

class X
{
    public function __get($name)
    {
        $this->{$name} = null;
        return $this->{$name};
    }
}

$x = new X();
var_dump(property_exists($x, 'foo')); // false
var_dump($x->foo); // NULL
var_dump(property_exists($x, 'foo')); // true

หรือบางทีคุณอาจใช้การสะท้อนกลับสำหรับคุณสมบัติ http://php.net/manual/en/class.reflectionproperty.php

หากคุณต้องการตรวจสอบ "isset" คุณต้องรู้จัก:

var_dump(isset($x), $x); // false + NULL with notice
$x = null;
var_dump(isset($x), $x); // false + NULL
unset($x);
var_dump(isset($x), $x); // false + NULL without notice

เมื่อคุณแน่ใจว่าเช็คในกรณีนี้ คุณสามารถใช้ isset()

แต่คุณควรตรวจสอบการมีอยู่ของทรัพย์สินก่อนเสมอ มิฉะนั้นคุณอาจมีพฤติกรรมที่ไม่ได้กำหนดไว้ในโค้ดของคุณ

person Deep    schedule 10.01.2016
comment
มันไม่เกี่ยวกับการตรวจพบว่ามีการกำหนด $this->bar[''anyStringValue หรือไม่ (อาจเป็นไปไม่ได้ด้วยซ้ำในการวิเคราะห์แบบคงที่) แต่เกี่ยวกับการตรวจพบว่า $this-›bar ไม่ได้ถูกกำหนดไว้ (อีกต่อไป) - person maxhb; 10.01.2016
comment
@maxhb คำตอบของฉันรวมถึงเกี่ยวกับเรื่องนี้ - person Deep; 10.01.2016

arrow_upward
2
arrow_downward

หลังจากการปรับโครงสร้างใหม่

เป็นการดีที่จะป้องกันสิ่งนี้ตั้งแต่แรก

คุณสามารถตรวจพบข้อผิดพลาดในการรีแฟคเตอร์ประเภทนี้ได้โดยการรันการทดสอบหลังจากขั้นตอนการรีแฟคเตอร์แต่ละขั้นตอนเท่านั้น ข้อผิดพลาดนี้จะปรากฏขึ้นเช่นกัน เนื่องจาก foo['blubb'] ถูกตั้งค่าเป็นค่าเฉพาะ และสิ่งนี้ควรทำให้เกิดผลที่ไม่พึงประสงค์ในการทดสอบอื่น ไม่ใช่แค่ในการทดสอบตรรกะของตัวตั้งค่าเท่านั้น

เราใช้แบบทดสอบ แต่บางครั้งคุณก็พลาด...

ใช่ เป็นเรื่องปกติที่ความคุ้มครองไม่สูงพอ นั่นเป็นเหตุผลว่าทำไมการทดสอบที่ครอบคลุมจึงเป็นจุดเริ่มต้นสำหรับการปรับโครงสร้างใหม่ทั้งหมด

สองบรรทัดนี้ไม่ใช่สีเขียวในรายงานความครอบคลุมของคุณ:

   if(!isset($this->bla['blubb'])) {
       $this->foo['blubb'] = $value;

นอกจากนี้ โปรดอย่าคิดเขียนทับวิธีการวิเศษเลย ฉันไม่ต้องการที่จะมีลักษณะ/นามธรรมอะไรก็ตามในทุกชั้นเรียน

คุณได้แยกมันออกไป แต่นั่นคือวิธีหนึ่งในการตรวจจับคุณสมบัติ: โดยใช้ฟังก์ชันเวทย์มนตร์ __set() (สำหรับ vars ที่ไม่สามารถเข้าถึงได้) หรือ property_exists() หรือใช้คลาส Reflection* เพื่อค้นหา

ตอนนี้มันสายเกินไปแล้ว คุณต้องการเครื่องมืออื่นที่จะตรวจจับข้อผิดพลาด ตกลง:

เครื่องมือนี้จะต้องแยกวิเคราะห์ไฟล์ PHP และพาเรนต์ของไฟล์ (เนื่องจากขอบเขตของตัวแปร) และค้นหา $this->bla โดยไม่ต้องมีการประกาศตัวแปร public|private|protected ก่อนหน้า (คุณสมบัติของคลาส) สิ่งนี้จะไม่ระบุประเภทข้อผิดพลาดที่แน่นอน เพียงแต่มีการเข้าถึง bla โดยไม่มีการประกาศ

เป็นไปได้ที่จะใช้สิ่งนี้เป็นกฎ CodeSniffer

คุณยังสามารถให้ http://phpmd.org/ หรือ https://scrutinizer-ci.com/ ลองดูสิ และในกรณีที่คุณใช้ PHP7: https://github.com/etsy/phan

tl;tr

การระบุข้อผิดพลาดที่แน่นอนและบริบทของมันมีความซับซ้อนโดยไม่ต้องรัน ประเมิน และวิเคราะห์โค้ดที่ซ่อนอยู่ ลองคิดถึงชื่อตัวแปรไดนามิกแล้วคุณจะรู้ว่าทำไม: คุณไม่รู้ด้วยซ้ำถึงชื่อของคุณสมบัติโดยดูที่ซอร์สโค้ด เพราะมันถูกสร้างขึ้นแบบไดนามิกระหว่างการไหลของโปรแกรม เครื่องวิเคราะห์แบบคงที่ไม่สามารถตรวจจับสิ่งนั้นได้

ตัววิเคราะห์แบบไดนามิกต้องติดตามทุกสิ่ง ที่นี่ $this-> เข้าถึงและจะคำนึงถึงบริบท: !isset(x) การประเมินบริบทสามารถค้นหาข้อผิดพลาดในการเขียนโค้ดทั่วไปได้มากมาย ในตอนท้ายคุณสามารถสร้างรายงานได้โดยบอกว่า $this->bla มีการเข้าถึงเพียง 1 ครั้งและนั่นบ่งชี้ว่า

  • มีการแนะนำคุณสมบัติที่ประกาศแบบไดนามิก แต่ไม่เคยนำมาใช้ซ้ำ โดยมีข้อเสนอแนะว่าคุณอาจทิ้งหรือประกาศเป็นคุณสมบัติของคลาส
  • หรือด้วยการประเมินบริบทเพิ่มเติม: เมื่อนั้นและเมื่อตัวแปรนี้ถูกเข้าถึงจากภายใน isset() - มีการเข้าถึงคีย์ที่ไม่มีอยู่ของคุณสมบัติที่ไม่ได้ประกาศ โดยไม่ต้องตั้งค่าล่วงหน้า () เป็นต้น
person Jens A. Koch    schedule 13.01.2016

arrow_upward
1
arrow_downward

ขณะนี้ในปี 2017 คุณกำลังมองหา เครื่องมือ PHPStan ฉันลิงก์คำนำสั้น ๆ ที่ฉันเขียนสำหรับผู้ใช้ครั้งแรก

มันทำสิ่งที่คุณต้องการอย่างแน่นอน!

person Tomas Votruba    schedule 21.05.2017