date:20190424

Re: [PHP-DEV] [RFC] Deprecate left-associative ternary operator

2019-04-24 Thread Sara Golemon

On Wed, Apr 24, 2019 at 6:48 PM Christoph M. Becker 
wrote:

> On 24.04.2019 at 19:25, Sara Golemon wrote:
>
> > On Tue, Apr 23, 2019 at 3:56 AM Nikita Popov 
> wrote:
> >
> >> 20 years of code in the wild has not "accepted that fact and moved on".
> If
> >> a left-associative ternary is used, it is almost certainly a bug. If
> people
> >> use this structure by accident (because it is familiar from other
> >> programming languages), I'd like them to get an error instead of having
> to
> >> figure out why their obviously correct code is not working or, in the
> worse
> >> case, just leave behind buggy code.
> >
> > I'm on dismal wifi at the moment, else I'd do some searches, but do you
> > have any examples of code in the wild subject to this bug?
> > I agree it's a lousy state, but it's not emergent.
>
> See .
>
>  Okay, so the most compelling part for me in this analysis is the fact
that Symfony and ZF have issues.  I can shrug off a lot of questionable PHP
coders, but i have to acknowledge major frameworks stumbling on this gotcha.

Convinced.
+1

-Sara

Re: [PHP-DEV] [RFC] [VOTE] Deprecate PHP's short open tags

2019-04-24 Thread Michael Kliewe

Hi there,

Am 24.04.2019 um 21:07 schrieb Peter Kokot:

Hello,

On Wed, 24 Apr 2019 at 19:44, Chase Peeler wrote:

On Wed, Apr 24, 2019 at 1:27 PM Marco Pivetta wrote:

Run a fixer: they are out there, and they are extremely stable too.

Also a good chance to finally take a look at code that has been rotting in
a hard drive for too much time.

All of that takes time though. I have 6,787 short opening tags found. Even
if I use a fixer to generate a diff, or, to fix them and then examine the
diff in a pull request... that's going to take a LOT of time. It's going to
start getting messy if I find false positives and need to exclude changes.
It still doesn't address the impact of changes that aren't found. Are you
100% positive that the fixers out there will catch EVERY single instance?
php-cs-fixer doesn't update
I've done few commits with about 8k files changed with very repeating
changes like this and without breaking anything. It will take you
about 30minutes... Let's say, one hour for also taking a break from
all the scrolling the git diff and to return the merge the next day
after another check or something like this.

The change (speaking for 8k files using short opening tags converted
to

Some thoughts and numbers from my side.

Nobody should not use a simple grep or sed like this:

grep -rin "It does not search in .phtml templates for example. And it has lots of
false-positives.

This is the output in one of my projects:

# grep -rin "./scripts/OpenInviter/autoupdate.php:97:
$start=0;$end=strpos($res,"
./scripts/_soap/_soap.php:1:./scripts/XXXupload.php:67: Text: value="">
./scripts/XXXupload.php:68: Text2: name="text2" value="">

Binary file ./library/phpillow/tests/phpillow/data/image_gif.gif matches
Binary file
./library/phpillow/tests/phpillow/data/.svn/text-base/image_gif.gif.svn-base
matches
./library/ezcomponents/Mail/src/tools.php:225: $pattern =
'/?$/';

./library//SoapApi.php:1:Binary file
./library/dompdf/lib/php-font-lib/sample-fonts/IntelClear-Light.ttf matches

Result:
2 false positives
3 results because "in the file path. Better check would be:

grep -rin "BUT: That simple grep did only find a small fraction of the problems,
for example .phtml templates were not scanned. Continue reading...

php-cs-fixer with default settings produced a diff file with 3403 lines,
51 line changes:
php php-cs-fixer-v2.phar fix --rules=full_opening_tag --diff --dry-run .
> result.txt
Reviewing these 51 changes took 3 minutes. No false positives here, all
of them were at the beginning of files.

BUT: it didn't search in .phtml files. php-cs-fixer by default only
processes .php and .phpt files (depending on the version you are using,
older versions also process .twig files as far as I can see). If you use
it, you may have to add other file extensions like .phtml, .tpl or
similar, depending on your project.

Running php-cs-fixer on .phtml files in that project results in a diff
file with 9527 lines, 1573 line changes just in the .phtml files of that
project. Whao...

I'm not sure if I'm alone, but I have lots of template code like this:

Xdata as $index => $data) { ?>
Something

Blindly replacing "
Xdata as $index => $data) { ?>
Something

The nice visual indenting of the "foreach" with 4 spaces is broken now,
and cannot be fixed, because "Yes, minor problem of cause, but it looked nice before, and now looks
broken and "wrongly indented".

Some random thoughts:
- What happens to .phar files? Do we have to scan the contents?
- What happens to "executable php files"? They don't have a file
extension, but a shebang "#!/usr/bin/env php", and potentially
short-open-tags in it.
- What happens if I miss to add a file extension to php-cs-fixer while
scanning and fixing code I'm not 100% familiar with? Code leak :-(
- What happens if I upgrade a server to PHP8, but do not check all
folders and projects on it?
- What happens if a Shared Hosting Provider increases the PHP version to
8 automatically for all its customers in the future? Does this happen in
the real world? Pages will not generate a 500 error, they will leak
code, with passwords in it...
Same with system administrators upgrading PHP to 8 without asking
code-owners/developers. Or migrating all projects blindly to a new
server with PHP8 installed. I guess that might happen. I guess, nobody
expects code leaks then...

Looks like a lot of work and problems for everybody... I didn't expect
the RFC to pass, thought that there are too many problems and lots of
work for no benefit :-( And I don't follow internals mailing list on a
daily basis, just check it "every few weeks for something important"...
In the past I was fine with BC breaks, for security, performance, or
similar. Even bigger ones (unable to detect like "Uniform Variablen
Syntax" for example). But this one looks heavy and dangerous for no
benefit...

64 matches

Mail list logo