Bug with passing arguments for non-tty stdin.

[Tseyang]

In https://github.com/ruby-syntax-tree/syntax_tree/pull/152/files#diff-9c0147bbfba8210004d93e758fd167a09f12f0749003f15bd8ea5f41da995f9bR390, I believe the conditional changed from

$stdin.tty? || arguments.any? to $stdin.tty? && (arguments.any? || options.scripts.any?). This seems to alter the conditional to prevent non-tty stdin from using FileItem types. Instead you get the following warning:

[warn] stdin
The listed files did not match the expected format.

This popped up for me when I tried to upgrade syntax_tree and it broke CI checks I had previously setup.

If this was intentional, is there a better recommended way to run stree:check in CI?

[kddnewton]

Yeah this broke and then was fixed again. Are you running on the latest version?

[Tseyang]

I was updating to 3.6.0

[xfalcox]

Oh, found the same bug in Discourse. Can't update to 3.6.0 because of this

discourse/discourse-chat#1256

[davidtaylorhq]

In case it's useful for anyone else - we unblocked ourselves at Discourse by faking a TTY in GitHub actions:

- name: Syntax Tree
  shell: script -qefc "/bin/bash {0}" /dev/null # fake tty to work around https://github.com/ruby-syntax-tree/syntax_tree/issues/159
  run: bundle exec stree check **/*.rb

[kddnewton]

Oh no! I'm so sorry to have broken your builds. I'm afraid I really don't understand exactly what TTY is and how it interacts with syntax tree if we're being totally honest. Would one of you be able potentially able to educate me?

[Tseyang]

So I think TTY shells are shells that accept interaction via text input/output. Forking a process/running stuff on CI generally is not TTY, since there's no interaction.

In this case, I don't think there's an issue with syntax_tree itself, but rather the CLI tooling (and Rake tasks that depend on it). The change/code I linked seems to indicate that for the CLI to accept a list of files to check, it used to accept files as long as $stdin was either TTY OR arguments was provided.

if $stdin.tty? || arguments.any?

However, in the change for #152, it made it such that files could only be passed in if a TTY $stdin was used AND either arguments are passed or options.scripts are passed

if $stdin.tty? && (arguments.any? || options.scripts.any?)

This means that non-TTY shells cannot pass filenames when running stree:check anymore (when they used to be able to). I think (have not had to chance to verify) a repro of the issue would be to try and run something like

SyntaxTree::Rake::CheckTask.new { |t| task.source_files = "foo.rb" }

stdout, stderr, status = Open3.capture3("./bin/rake stree:check") # this uses a non-TTY shell
puts stdout
puts stderr

I was not sure if it was intentional for the TTY shell restriction to be introduced.

[kddnewton]

Ahh thank you very much, that helps a lot.

Previously, I was checking if STDIN was a TTY to determine if there was content to be read. Instead now I'll just check the opposite (that inline scripts or filepaths were passed) and if nothing is present I'll try to read STDIN. The PR is up and I'll release it when it merges.

[kddnewton]

Thank you for your patience, v3.6.1 is out which fixes this. Let me know if you experience any more trouble.

[davidtaylorhq]

3.6.1 works great - thanks @kddnewton

[Tseyang]

thanks for the turnaround @kddnewton . Am a fan of syntax_tree so far, keep up the great work!