8000 [Console] Wrong documentation about section() · Issue #9927 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

[Console] Wrong documentation about section() #9927

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.

Sign up for GitHub

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

Jump to bottom
Closed
Aerendir opened this issue Jun 15, 2018 · 7 comments
Closed

[Console] Wrong documentation about section() #9927

Aerendir opened this issue Jun 15, 2018 · 7 comments
Labels
bug Console help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review
Milestone
4.1

Comments

@Aerendir
Copy link
Contributor
Aerendir commented Jun 15, 2018 •
edited
Loading

The documentation about Console Sections is wrong.

The documentation uses this example:

class MyCommand extends Command
{
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $section1 = $output->section();
        $section2 = $output->section();
        $section1->writeln('Hello');
        $section2->writeln('World!');
        // Output displays "Hello\nWorld!\n"

        // overwrite() replaces all the existing section contents with the given content
        $section1->overwrite('Goodbye');
        // Output now displays "Goodbye\nWorld!\n"

        // clear() deletes all the section contents...
        $section2->clear();
        // Output now displays "Goodbye\n"

        // ...but you can also delete a given number of lines
        // (this example deletes the last two lines of the section)
        $section1->clear(2);
        // Output is now completely empty!
    }
}

The problem is that the interface OutputInterface doesn't have any section() method.

The section() method, instead, is in ConsoleOutputInterface.

I don't know why it is a DocComment and not a real method, but anyway, the examples of the documentation are wrong, as the interface to use is ConsoleOutputInterface and not OutputInterface.

The problem is that type hinting the execute() method with the interface ConsoleOutputInterface is not allowed by PHP.

I don't know where this should be discussed, but is something that should be addressed as to use the section() method someone has to dig in the code to understand where this method is.

This is the PR that updated the documentation: #9462

The same "errors" are also in the blog post that announced the new feature: https://symfony.com/blog/new-in-symfony-4-1-advanced-console-output
@xabbuh xabbuh added this to the 4.1 milestone Jun 15, 2018
@javiereguiluz
Copy link
Member

I've just tested this same code in a 4.1 app (the Symfony Demo in fact) and it worked for me. Do you see an exception when using this code in your app?

@Aerendir
Copy link
Contributor Author

@javiereguiluz , no, I don't see any exception: the code works perfectly.

The problem is that the IDE (PHPStorm) signals errors in the type hinting and the static analysis, too.

It works because the consoole object implements both interfaces, but the type hinting is on the OutputInterface interface and is not possible to type hint using ConsoleOutputInterface (that contains the section() method because of inheritance from Command class.

So, the method is concretely available, but the IDE (nor the static analysis tools) cannot find it as we type hint on the OutputInterface and not on the ConsoleOutput object itself (that implements both ConsoleOutputInterface and OutputInterface.

@javiereguiluz javiereguiluz added the help wanted Issues and PRs which are looking for volunteers to complete them. label Jun 20, 2018
@xabbuh
Copy link
Member
xabbuh commented Jun 22, 2018

This could indeed lead to problems when writing tests for your commands where you will probably not pass a ConsoleOutput instance, right?

@Aerendir
Copy link
Contributor Author

@xabbuh , I've not written tests at the moment: I don't know if it will cause problems... Anyway, I think that no: I will not mock OutputInterface as it hasn't section() method... I didn't think about tests, but yes, it is a problem with them...

@Aerendir Aerendir mentioned this issue Jul 19, 2018
Closed
@chalasr
Copy link
Member
chalasr commented Oct 10, 2018

Yea, you need an instanceof check to avoid warnings from static code analyzers.
This issue is similar to symfony/symfony#20258 (the SessionInterface and getFlashBag() topic), nothing to fix on the docs for sure, and probably a won't fix in core.

@xabbuh The output used for command testing is a ConsoleOutput when using CommandTester and ApplicationTester, there should be no issue

@javiereguiluz
Copy link
Member

Thanks @chalasr for your comments. I'm closing this issue accordingly. Thanks!

@Aerendir
Copy link
Contributor Author
Aerendir commented Oct 18, 2018 •
edited
Loading

@chalasr , yes, you are right, I know this: this is sufficient for the static analysis.

Unfortunately, this forces me to not set a method parameter type hinting, and this causes SymfonyInsight to complain about this as it suggests to add the type hint.

Now, I could also simply hide the issue on SymfonyInsights side, but this is a kind of blindness and not a real fix.

@javiereguiluz , please, reopen the issue as it is not solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Console help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review
Projects
None yet
Milestone
4.1
Development

No branches or pull requests
5 participants
@javiereguiluz @xabbuh @Aerendir @chalasr @carsonbot
0