From b035abc6d311a7ffd1838f1b94aa8a3a05bc88c6 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Mon, 7 Apr 2008 04:42:35 +0000 Subject: [PATCH] #2380 - ManifestBuilder shouldn't recognise class definitions in comments and strings as real git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@52239 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- core/ManifestBuilder.php | 20 ++++++++- tests/ManifestBuilderTest.fixture.inc | 36 +++++++++++++++ tests/ManifestBuilderTest.php | 65 ++++++++++++++++++++++----- 3 files changed, 109 insertions(+), 12 deletions(-) diff --git a/core/ManifestBuilder.php b/core/ManifestBuilder.php index 393708d3d..3f8995dba 100644 --- a/core/ManifestBuilder.php +++ b/core/ManifestBuilder.php @@ -171,7 +171,7 @@ class ManifestBuilder { $manifestInfo["require_once"][] = "$baseDir/$filename/_config.php"; // Include this so that we're set up for connecting to the database // in the rest of the manifest builder - require_once("$baseDir/$filename/_config.php"); + require("$baseDir/$filename/_config.php"); } } @@ -378,6 +378,22 @@ class ManifestBuilder { if(!$file) die("Couldn't open $filename
"); + // Remove comments from $file so that we don't make use of a class-def inside a comment + $file = preg_replace('/\/\/.*([\n\r])/','$1', $file); + $file = preg_replace('/\/\*.*\*\//Us','', $file); + + // Remove strings from $file so that we don't make use of a class-def inside a strin + $file = str_replace(array("\\'",'\\"'), "{! ESCAPED QUOTE !}", $file); + $file = preg_replace("/'[^']*'/s",'', $file); + $file = preg_replace('/"[^"]*"/s','', $file); + + // Remove heredoc strings from $file so that we don't make use of a class-def inside a strin + if(preg_match_all('/<<<(.*)/', $file, $heredocs)) { + foreach($heredocs[1] as $code) { + $file = preg_replace('/<<<' . $code . '\n.*\n' . $code . '[\n;]/s', '', $file); + } + } + $classes = array(); $size = preg_match_all('/class (.*)[ \n]*{/m', $file, $classes); @@ -535,4 +551,4 @@ class ManifestBuilder { } -?> +?> \ No newline at end of file diff --git a/tests/ManifestBuilderTest.fixture.inc b/tests/ManifestBuilderTest.fixture.inc index fb4d248e4..3d602809f 100644 --- a/tests/ManifestBuilderTest.fixture.inc +++ b/tests/ManifestBuilderTest.fixture.inc @@ -15,10 +15,13 @@ PHP 'sapphire/MyClass.php' => << PHP , diff --git a/tests/ManifestBuilderTest.php b/tests/ManifestBuilderTest.php index fb60a5424..c39ae7c88 100644 --- a/tests/ManifestBuilderTest.php +++ b/tests/ManifestBuilderTest.php @@ -3,22 +3,15 @@ class ManifestBuilderTest extends SapphireTest { function testManifest() { $baseFolder = TEMP_FOLDER . '/manifest-test'; - - global $_CLASS_MANIFEST, $project; - $originalClassManifest = $_CLASS_MANIFEST; - $originalProject = $project; - $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); - Debug::show($manifestInfo); $this->assertEquals("$baseFolder/sapphire/MyClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['MyClass']); $this->assertEquals("$baseFolder/sapphire/subdir/SubDirClass.php", $manifestInfo['globals']['_CLASS_MANIFEST']['SubDirClass']); $this->assertNotContains('OtherFile', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); - global $_CLASS_MANIFEST, $project; - $project = $originalProject; - $_CLASS_MANIFEST = $originalClassManifest; - + $this->assertContains('MyClass', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertContains('MyClass_Other', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertContains('MyClass_Final', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); // Check aspects of PHP file $manifest = ManifestBuilder::generate_php_file($manifestInfo); @@ -32,6 +25,50 @@ class ManifestBuilderTest extends SapphireTest { $this->assertEquals(1, preg_match('/require_once\("[^"]+rahbeast\/_config.php"\);/i', $manifest), "rahbeast/_config.php included"); $this->assertEquals(1, preg_match('/require_once\("[^"]+sapphire\/_config.php"\);/i', $manifest), "sapphire/_config.php included"); } + + + function testManifestIgnoresClassesInComments() { + $baseFolder = TEMP_FOLDER . '/manifest-test'; + $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); + + /* Our fixture defines the class MyClass_InComment inside a comment, so it shouldn't be included in the class manifest. */ + $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); + $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); + $this->assertNotContains('MyClass_InComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['hastable'])); + + /* Our fixture defines the class MyClass_InSlashSlashComment inside a //-style comment, so it shouldn't be included in the class manifest. */ + $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); + $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); + $this->assertNotContains('MyClass_InSlashSlashComment', array_keys($manifestInfo['globals']['_ALL_CLASSES']['hastable'])); + } + + function testManifestIgnoresClassesInStrings() { + $baseFolder = TEMP_FOLDER . '/manifest-test'; + $manifestInfo = ManifestBuilder::get_manifest_info($baseFolder); + + /* If a class defintion is listed in a single quote string, then it shouldn't be inlcuded. Here we have put a class definition for MyClass_InSingleQuoteString inside a single-quoted string */ + $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); + $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); + $this->assertNotContains('MyClass_InSingleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['hastable'])); + + /* Ditto for double quotes. Here we have put a class definition for MyClass_InDoubleQuoteString inside a double-quoted string. */ + $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); + $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); + $this->assertNotContains('MyClass_InDoubleQuoteString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['hastable'])); + + /* Finally, we need to ensure that class definitions inside heredoc strings aren't included. Here, we have defined the class MyClass_InHeredocString inside a heredoc string. */ + $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_CLASS_MANIFEST'])); + $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['exists'])); + $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['parents'])); + $this->assertNotContains('MyClass_InHeredocString', array_keys($manifestInfo['globals']['_ALL_CLASSES']['hastable'])); + } + + + protected $originalClassManifest, $originalProject; function setUp() { include('tests/ManifestBuilderTest.fixture.inc'); @@ -62,9 +99,17 @@ class ManifestBuilderTest extends SapphireTest { } } } + + global $_CLASS_MANIFEST, $project; + $this->originalClassManifest = $_CLASS_MANIFEST; + $this->originalProject = $project; } function tearDown() { + global $_CLASS_MANIFEST, $project; + $project = $this->originalProject; + $_CLASS_MANIFEST = $this->originalClassManifest; + // Kill the folder after we're done $baseFolder = TEMP_FOLDER . '/manifest-test/'; Filesystem::removeFolder($baseFolder);