bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Bozhidar Batsov]

In the following snippet `format` is font-locked as the built-in
function, but it's actually a variable.

respond_to do |format|
  format.json do
    # ...
  end
end

This problem obviously exists for all built-ins, but it's not a big
issue in general, since most of them don't have names that are likely to
be used as variable names. Not sure if we can have a simple solution for
this problem without a parser, though. I guess we can have a check if
the built-in methods have arguments (as most of them have). 




In GNU Emacs 24.3.50.1 (x86_64-apple-darwin13.0.0, NS apple-appkit-1265.00)
 of 2014-01-27 on Bozhidars-MacBook-Pro.local
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --prefix=/usr/local/Cellar/emacs/HEAD --without-dbus
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/HEAD/share/info/emacs
 --without-gnutls --with-ns --disable-ns-self-contained'

Important settings:
  locale-coding-system: utf-8-unix

Major mode: Ruby

Minor modes in effect:
  diff-auto-refine-mode: t
  subword-mode: t
  ruby-tools-mode: t
  inf-ruby-minor-mode: t
  guru-mode: t
  erc-truncate-mode: t
  erc-spelling-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  which-function-mode: t
  flx-ido-mode: t
  ido-ubiquitous-mode: t
  winner-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  whitespace-mode: t
  global-anzu-mode: t
  anzu-mode: t
  projectile-global-mode: t
  projectile-mode: t
  flyspell-mode: t
  volatile-highlights-mode: t
  global-hl-line-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  savehist-mode: t
  show-smartparens-global-mode: t
  show-smartparens-mode: t
  smartparens-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  prelude-global-mode: t
  prelude-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
k e l y SPC o p e r a t o r s SPC w i t h SPC s p e
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> C-x b <return>
<backspace> * <return> <tab> <down-mouse-1> <mouse-1>
<backspace> = = <return> <tab> <backspace> <backspace>
<backspace> \ <return> C-p C-p C-n <backspace> / <backspace>
/ <return> <help-echo> <help-echo> <help-echo> <help-echo>
C-x b <return> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> O t h e r SPC s
i m i l a r SPC o p e r a t o r s SPC l i k e SPC *
, SPC / , SPC e t c SPC e x h i b i t SPC t h e SPC
s a m e SPC p r o b l e m . C-p C-p <down-mouse-1>
<mouse-1> C-x b <return> <backspace> <backspace> +
<return> x SPC = SPC <backspace> <backspace> = SPC
5 SPC + SPC 1 0 C-a C-f C-f C-f C-f C-k y SPC + SPC
z C-a C-SPC C-e M-w C-x o <return> <return> # # # <backspace>
<backspace> <backspace> <backspace> C-y C-a SPC SPC
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC
SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC
SPC SPC SPC SPC C-e SPC # SPC t h i s SPC s h o u l
d SPC n o t SPC b e SPC i n <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> b a
d SPC i n d e n t a t i o n C-c C-c y <help-echo> <help-echo>
<help-echo> <help-echo> M-x <return>

Recent messages:
Saving file /Users/bozhidar/projects/test.rb...
Wrote /Users/bozhidar/projects/test.rb
Mark set
Auto-saving...done
Mark set
Send this bug report to the Emacs maintainers? (y or n)  y
Sending...
Mark set [2 times]
Sending via mail...
Sending...done

Load-path shadows:
/Users/bozhidar/.emacs.d/elpa/flycheck-20140121.147/.dir-locals hides /usr/local/Cellar/emacs/HEAD/share/emacs/24.3.50/lisp/gnus/.dir-locals
/Users/bozhidar/.emacs.d/elpa/tabulated-list-20120406.2251/tabulated-list hides /usr/local/Cellar/emacs/HEAD/share/emacs/24.3.50/lisp/emacs-lisp/tabulated-list

Features:
(shadow sort emacsbug ielm pcase mail-extr sgml-mode nxml-uchnm rng-xsd
xsd-regexp rng-cmpct vc-annotate vc vc-dispatcher lisp-mnt
network-stream starttls tls cider cider-mode cider-repl cider-eldoc
cider-version clojure-test-mode cider-interaction arc-mode archive-mode
cider-client nrepl-client cider-util ewoc clojure-mode-expansions
clojure-mode inf-lisp ibuf-ext ibuffer conf-mode yaml-mode ace-jump-mode
find-dired ffap url-parse url-vars hippie-exp misearch multi-isearch
eieio-opt speedbar sb-image ezimage dframe mule-util markdown-mode
noutline outline executable magit-key-mode magit view epa derived epg
epg-config diff-mode git-rebase-mode git-commit-mode server log-edit
message sendmail rfc822 mml mml-sec mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev mail-utils
gmm-utils mailheader pcvs-util add-log vc-git superword subword
ruby-tools inf-ruby ruby-mode-expansions smartparens-ruby ruby-mode smie
rainbow-mode color rainbow-delimiters elisp-slime-nav guru-mode
prelude-key-chord key-chord prelude-xml nxml-mode-expansions
html-mode-expansions smartparens-html rng-nxml rng-valid rng-loc rng-uri
rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok
prelude-web prelude-scss prelude-scheme prelude-ruby prelude-perl
prelude-org prelude-js prelude-erc erc-truncate erc-autoaway
erc-spelling erc-notify erc-log erc-list erc-menu erc-join erc-ring
erc-networks erc-pcomplete erc-track erc-match erc-button erc-fill
erc-stamp erc-netsplit erc-goodies erc erc-backend erc-compat
prelude-emacs-lisp prelude-css prelude-common-lisp slime-autoloads
prelude-clojure prelude-lisp prelude-c prelude-programming flycheck
help-mode rx f which-func imenu prelude-ido smex flx-ido flx
ido-ubiquitous warnings ido prelude-osx exec-path-from-shell
prelude-global-keybindings prelude-editor winner undo-tree diff esh-var
esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg eldoc esh-groups eshell
esh-module esh-mode esh-util re-builder whitespace browse-kill-ring
midnight ediff-merg ediff-wind ediff-diff ediff-mult ediff-help
ediff-init ediff-util ediff dired-x dired anzu projectile pkg-info
find-func grep compile s bookmark pp expand-region text-mode-expansions
er-basic-expansions expand-region-custom expand-region-core flyspell
ispell etags volatile-highlights hl-line windmove tramp-cache tramp-sh
tramp tramp-compat auth-source gnus-util mm-util mail-prsvr
password-cache tramp-loaddefs trampver shell pcomplete comint ansi-color
ring format-spec recentf tree-widget wid-edit savehist saveplace
diminish smartparens-config smartparens autorevert filenotify delsel
prelude-mode easy-mmode edmacro kmacro prelude-core epl advice help-fns
dash thingatpt prelude-ui zenburn-theme prelude-packages finder-inf
ack-and-a-half-autoloads diminish-autoloads erlang-autoloads
flx-ido-autoloads eieio byte-opt bytecomp byte-compile cconv eieio-core
grizzl-autoloads key-chord-autoloads logito-autoloads info easymenu
cl-macs gv move-text-autoloads paredit-autoloads pkg-info-autoloads
puppet-mode-autoloads rainbow-delimiters-autoloads request-autoloads
rubocop-autoloads volatile-highlights-autoloads yaml-mode-autoloads
package cl cl-loaddefs cl-lib time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
cocoa ns multi-tty emacs)

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Dmitry Gutov]

Bozhidar Batsov <bozhidar@batsov.com> writes:

> In the following snippet `format` is font-locked as the built-in
> function, but it's actually a variable.
>
> respond_to do |format|
>   format.json do
>     # ...
>   end
> end
>
> This problem obviously exists for all built-ins, but it's not a big
> issue in general, since most of them don't have names that are likely to
> be used as variable names. Not sure if we can have a simple solution for
> this problem without a parser, though. I guess we can have a check if
> the built-in methods have arguments (as most of them have).

A lot of them don't, though (or have no required arguments): __callee__,
__dir__, __method__, binding, caller, block_given?, exit, exit!, raise,
(s)rand, readline(s), sleep.

Guess we can separate these, and for all others, check if they're
followed by " *[]|,.)}]\\|$".

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Bozhidar Batsov]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

On Saturday, February 1, 2014 at 5:19 PM, Dmitry Gutov wrote:
> Bozhidar Batsov <bozhidar@batsov.com (mailto:bozhidar@batsov.com)> writes:
>  
> > In the following snippet `format` is font-locked as the built-in
> > function, but it's actually a variable.
> >  
> > respond_to do |format|
> > format.json do
> > # ...
> > end
> > end
> >  
> > This problem obviously exists for all built-ins, but it's not a big
> > issue in general, since most of them don't have names that are likely to
> > be used as variable names. Not sure if we can have a simple solution for
> > this problem without a parser, though. I guess we can have a check if
> > the built-in methods have arguments (as most of them have).
> >  
>  
>  
> A lot of them don't, though (or have no required arguments): __callee__,
> __dir__, __method__, binding, caller, block_given?, exit, exit!, raise,
> (s)rand, readline(s), sleep.
>  
> Guess we can separate these, and for all others, check if they're
> followed by " *[]|,.)}]\\|$".
>  
>  

Separating the methods sounds reasonable. Variables can’t end with ?, so block_given? is always clear. Same goes for exit!. Pretty sure no one will ever name a var __xxx__. `binding` & `caller` seem the most likely names in the list to cause a problem like `format`, since they’d be good variable names (at least in code making use of metaprogramming).  


[-- Attachment #2: Type: text/html, Size: 1992 bytes --]

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Dmitry Gutov]

On 01.02.2014 17:31, Bozhidar Batsov wrote:
> Variables can’t end with ?, so
> block_given? is always clear. Same goes for exit!. Pretty sure no one
> will ever name a var __xxx__.

Good points.

> `binding` & `caller` seem the most likely
> names in the list to cause a problem like `format`, since they’d be good
> variable names (at least in code making use of metaprogramming).

Still not very likely, though (e.g. compared to `format`, which is in 
every other Rails tutorial). And anyone doing metaprogramming should be 
aware of these two, and would probably name their vars differently.

I have a patch ready, but here's what I'm not sure about: if the 
built-in method that requires arguments is followed by (maybe whitespace 
and) newline, do we highlight it?

If yes, we'll have false positives when a local variable with that name 
is at a line by itself, maybe on the last line of a method, so that its 
value is returned.

If no, any such built-in won't be highlighted until the user types in 
the first argument, which is also suboptimal (if I made a typo, I'll 
have to go back a longer distance).

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Bozhidar Batsov]

[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]

On Saturday, February 1, 2014 at 6:29 PM, Dmitry Gutov wrote:
> On 01.02.2014 17:31, Bozhidar Batsov wrote:
> > Variables can’t end with ?, so
> > block_given? is always clear. Same goes for exit!. Pretty sure no one
> > will ever name a var __xxx__.
> >  
>  
>  
> Good points.
>  
> > `binding` & `caller` seem the most likely
> > names in the list to cause a problem like `format`, since they’d be good
> > variable names (at least in code making use of metaprogramming).
> >  
>  
>  
> Still not very likely, though (e.g. compared to `format`, which is in  
> every other Rails tutorial). And anyone doing metaprogramming should be  
> aware of these two, and would probably name their vars differently.
>  
> I have a patch ready, but here's what I'm not sure about: if the  
> built-in method that requires arguments is followed by (maybe whitespace  
> and) newline, do we highlight it?
>  
> If yes, we'll have false positives when a local variable with that name  
> is at a line by itself, maybe on the last line of a method, so that its  
> value is returned.
>  
> If no, any such built-in won't be highlighted until the user types in  
> the first argument, which is also suboptimal (if I made a typo, I'll  
> have to go back a longer distance).
>  
>  

I’d say that the second option is better, since it won’t cause false positives.  
This would also take care of an annoying bit regarding the `p` built-in - each time you start
typing something that starts with “p" the “p” briefly gets font-locked as built-in, before the following letter is inserted.  


[-- Attachment #2: Type: text/html, Size: 2511 bytes --]

bug#16610: 24.3.50; [ruby-mode] Variables named as built-in functions font-locked incorrectly

[Dmitry Gutov]

Version: 24.4

On 01.02.2014 18:47, Bozhidar Batsov wrote:
> I’d say that the second option is better, since it won’t cause false
> positives.
> This would also take care of an annoying bit regarding the `p` built-in
> - each time you start
> typing something that starts with “p" the “p” briefly gets font-locked
> as built-in, before the following letter is inserted.

Yeah, okay. I haven't noticed this problem with "p" yet, but probably 
would, from now on.

Installed, with the caveat that the regexp doesn't take care of binary 
operations and keywords. Have to leave space for future improvement. :)