Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not valid PHP namespaces #637

Closed
Ralle opened this issue Oct 16, 2019 · 6 comments · Fixed by kaitai-io/kaitai_struct_compiler#190
Closed

Not valid PHP namespaces #637

Ralle opened this issue Oct 16, 2019 · 6 comments · Fixed by kaitai-io/kaitai_struct_compiler#190
Milestone

Comments

@Ralle
Copy link

Ralle commented Oct 16, 2019

I am getting a lot of errors from the PHP generated by kaitai.

Fatal error: Uncaught Error: Undefined constant 'FunctionInfo' in /Users/ralle/Code/Warcraft3Parsers/dist/FunctionInfo.php:5258

On that line is this:

namespace \FunctionInfo;

The first bug here is that the namespace construct always comes with an absolute namespace, so the \ does not make sense.

When removing the \ you get this:

Fatal error: Namespace declaration statement has to be the very first statement or after any declare call in the script in /Users/ralle/Code/Warcraft3Parsers/dist/FunctionInfo.php on line 5258

In PHP you want the namespace before everything else. Moving the namespace to the first line fixes it.

The only problem now is that the main object, FunctionInfo is now in the FunctionInfo namespace. So the fix here is to change all references to it (inside the same file to just FunctionInfo instead of \FunctionInfo.

But in other files that refer to FunctionInfo, we want them to either have this in the top just after the namespace:

use FunctionInfo\FunctionInfo;

or refer to it as \FunctionInfo\FunctionInfo unless they have the same namespace.

@GreyCat
Copy link
Member

GreyCat commented Oct 27, 2019

I don't have much experience with PHP — cc @markbook2 — any chance you could take a look at this one?

Otherwise, @Ralle, can I ask you to add contribute some test into our test repo that clearly demonstrates the problem, or explain how to reproduce it?

@Ralle
Copy link
Author

Ralle commented Oct 28, 2019

First of all, on this line:

https://github.com/kaitai-io/kaitai_struct_compiler/blob/407389a870d17b8638615c568118bc28f8f227b8/shared/src/main/scala/io/kaitai/struct/languages/PHPCompiler.scala#L46

You need to check if config.phpNamespace was actually set to anything, otherwise it will result in a namespace \SomeNameSpace which is invalid. Namespaces in namespace blocks are always absolute so they don't start with a slash.

As you cannot change namespace during a file, I think you have to make it so if nsPart is nonEmpty, it will simply not output a namespace.

You can reproduce it by putting a type intypes:. This type will have its own namespace block above its definition which is invalid PHP. It has to be at the start of the file and nowhere else.

@ghost
Copy link

ghost commented Nov 25, 2019

@Ralle Please provide the expected PHP code and fragment of the generated invalid PHP code. The 10-20 lines will be enough I think. Thanks.

@andrewDyakin
Copy link

namespace \DosMz;

should be

namespace DosMz;

@ghost
Copy link

ghost commented Jan 3, 2020

@GreyCat
If the generated .php files have the \ at the begging of the namespace name, then it should be fixed, i.e. the following:

namespace \Foo\Bar\Baz;

should be replaced with:

namespace Foo\Bar\Baz;

It should be done only for the namespace definition.

@generalmimon
Copy link
Member

generalmimon commented Jan 3, 2020

As you cannot change namespace during a file

This type will have its own namespace block above its definition which is invalid PHP. It has to be at the start of the file and nowhere else.

This is not true. Just open generated code of any passing PHP test with some subtypes (e.g. NestedTypes). It's absolutely possible to define multiple namespaces in the same PHP file, although it's illegal to mix namespaced code and non-namespaced when using "semicolon syntax".

Actually, if you compile the NestedTypes format with no specified PHP namespace, it produces a code with this structure:

class NestedTypes {}
namespace NestedTypes {
  class SubtypeA {}
  class SubtypeB {}
}
namespace NestedTypes\SubtypeA {
  class SubtypeC {}
}

The NestedTypes class is global, but all subtypes are in some namespace. The PHP documentation says: "To combine global non-namespaced code with namespaced code, only bracketed syntax is supported." So the current codegen is incorrect, it is invalid to define NestedTypes class without any previous namespace definition. To be able to embed global code to file with some namespace declarations, it's necessary to wrap it into namespace {...}.

So I had to convert all namespace definitions to bracketed to make this work. I ran all tests locally for PHP, only had to fix enums declaration, but I think everything seems fine. Could you have a look, @GreyCat?

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

Successfully merging a pull request may close this issue.

4 participants