• Resolved m_i_n

    (@m_i_n)


    In a theme code, I’ve got my own class with the following methods:

    class SomeClassName {
    
    	protected function __() {
    		...
    	}
    
    	protected function _x() {
    		...
    	}
    
    }

    And Theme-check reports:

    WARNING: Found a translation function that is missing a text-domain. Function _x, with the arguments
    WARNING: Found a translation function that is missing a text-domain. Function __, with the arguments

    which is not true.

    https://www.ads-software.com/plugins/theme-check/

Viewing 12 replies - 1 through 12 (of 12 total)
  • Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    Why would you name your functions that? My advice would be to not do that, essentially.

    Thread Starter m_i_n

    (@m_i_n)

    It’s part of my WPML compatibility layer. The code is correct and works totally fine, and naming is like that on purpose, because it’s related to WordPress’ __(). It’s not in conflict with it in any way. These are private methods of a class (and in a separated namespace).

    My advice would be to not do that, essentially.

    Still, it’s a Theme check’s bug. Just add some negative lookbehind to your regular expression and it will be fixed.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    Yeah, but the problem is that you’re going to run into other cases where that naming structure will cause problems. Our multilingual scanning tools will fail on it too. Things like POEdit and such won’t expect that sort of thing.

    Essentially, I’m inclined to not fix this in theme-check because of various compatibility breakage. This *should* fail, in my opinion. In fact, I would consider adding explicit checks to force this to fail.

    This isn’t just about theme-check. You should not use those function names. They’re special, reserved, cases.

    Again, why are you naming these functions in this way? These function names have special uses for a reason. Duplicating them will cause problems unless you have valid reasons for doing so.

    Thread Starter m_i_n

    (@m_i_n)

    This nomenclature doesn’t cause any problems, because these are methods inside a class, in a separated namespace (that’s what namespace are for).

    POEdit doesn’t have any problem with that code – it doesn’t detect these methods, and just __() functions, etc. Exactly how it is suppose to.

    It doesn’t matter why I’ve named them this way (analogy to WordPress functions, etc.), it matters the code is totally correct from PHP perspective, doesn’t cause any conflicts with WP, WPML, POEdit and any other third party software except Theme-check ??

    Don’t take me wrong – it doesn’t matter to me to much if you fix it or nor – I just report a bug. And reasons why I use this code and not another, is a separate discussion about coding practices, which is irrelevant to the bug itself.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    I understand that. However, if you are making this theme to be submitted here, to this site, then you’re likely going to run into problems beyond simply theme-check.

    We scan themes for things like __() and _X() too, and those feed into GlotPress. I cannot guarantee that your functions will work in that existing system. Best to avoid the problem entirely, really.

    Thread Starter m_i_n

    (@m_i_n)

    I agree, using these kind of method names may be controversial, but anyway, Theme check wrongly states “Found a translation function that is missing a text-domain. Function _x, with the arguments”. It’s just not true ??

    BTW, it would be helpful if Theme-check would report a line number or at least file name where it found a problem. It’s sometimes hard to find the problem without it, especially a false positive.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    It gives you the arguments, which should be enough for you to find the issue. It can’t give you a line number because it’s reading a tokenized version of your code to find the calls in the first place.

    And it is not a false positive. The function is named _x and it has the incorrect arguments.

    This will not be fixed in theme check. Rename your functions.

    Thread Starter m_i_n

    (@m_i_n)

    And it is not a false positive. The function is named _x and it has the incorrect arguments.

    It’s not a function call, it is a declaration of class method. These are two different things.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    I understand that, but it’s named the same as a function which is a special case. You need to rename the function.

    Themes containing this code will cause breakage in various places. I’m going to have to not allow it, and probably will add a check to prevent it specifically.

    Regarding Theme Check, it is not intended to be perfect, or correct at all times. It’s a first line of defense. No code scanner is, or ever will be, perfect.

    Thread Starter m_i_n

    (@m_i_n)

    No, it won’t cause breakage in any places. That’s the point of PHP namespaces.

    I use this code concept in 3 thousands active theme instances and never had any problem related to it. WP, POEdit, WPML, Polylang… it all works just fine because the code is 100% valid. Only Theme-check and you have problem with it.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    Namespaces themselves break sites running PHP 5.2, but you’re missing my point.

    I did not say that this would break your site. I did not say that it would not work. I did not even say that it’s an invalid way to do things.

    What I said was that it would cause breakage. I did not specify where that breakage would occur.

    Understand that these localization functions are special cases. They have uses OUTSIDE of just the context of PHP. Things like POEdit read files to find them. Therefore, calls to __() and such have to obey special rules that are above and beyond simply “working” in a PHP context.

    Just because POEdit is fine with your usage here does not mean that every possible code scanner out there will be. Our code scanners here on www.ads-software.com also look for function calls like __() and if you’re using that function name in a different way, then it’s possible that we’ll pick up your different usage incorrectly. Because we’re not actually *running* your code, we’re reading it as a text file.

    Plugin Author Samuel Wood (Otto)

    (@otto42)

    www.ads-software.com Admin

    I’ll give you an example. This is wrong:

    _e( 'String' . 'Translation', 'domain' );

    Now, do you see why that is wrong? It’s perfectly valid PHP. But a code scanner can’t see that the string to be translated here is “StringTranslation” because of the concatenation.

    This has nothing whatsoever to do with whether or not it works or whether or not it is valid PHP code. This has to do with following an established convention intended to make things interoperable.

    A naive code scanner would simply look for __(‘thing’) and parse out the ‘thing’ part to put into a translation file. But now, you want to be able to call $yourclass->__( whatever ), which means that code scanners have to avoid finding your custom calls. They need to now know to ignore ->__(, which is new.

    You’re overloading function names in a way that is valid PHP, but which may not be accounted for by existing code scanners. That’s a bad-idea.

Viewing 12 replies - 1 through 12 (of 12 total)
  • The topic ‘"Function __, with the arguments" – false positive’ is closed to new replies.