Skip to content
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.

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

PuppetManifest parser does not recognize definitions if the name contains underscore. #1901

Closed
ahakanbaba opened this issue Oct 4, 2018 · 8 comments

Comments

@ahakanbaba
Copy link
Contributor

ahakanbaba commented Oct 4, 2018

The name of the parser: puppetManifest

The command line you used to run ctags:

$ echo example.pp | ctags --options=NONE  -L -

The content of input file: example.pp

define one_one::two($ensure) {
    file { "/tmp/fqdefinition": ensure => $ensure }
}

The tags output you are not satisfied with:

empty output.

The tags output you expect:

/tmp/fqdefinition	example.pp	/^    file { "\/tmp\/fqdefinition": ensure => $ensure }$/;"	r	definition:one_one::two
one_one::two	example.pp	/^define one_one::two($ensure) {$/;"	d

The version of ctags:

$ ctags --version
Universal Ctags 0.0.0, Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Oct  3 2018, 22:35:30
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +iconv, +option-directory, +xpath

How do you get ctags binary:

Build it locally. At this commit.
Following the instructions from here

On a centos7 machine

$ uname -a
Linux <hostname> 3.10.0-862.9.1.el7.x86_64 #1 SMP Mon Jul 16 16:29:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
@ahakanbaba
Copy link
Contributor Author

ahakanbaba commented Oct 4, 2018

The example is very similar to the already existing test case about fully qualified defines here .

If you put an underscore anywhere in the name (one_one::two or one::two_two) the parser cannot find any tags.

@ahakanbaba
Copy link
Contributor Author

ahakanbaba commented Oct 4, 2018

It looks like @masatake has spent a lot of time on the puppetParser. ^ FYI.

@masatake
Copy link
Member

masatake commented Oct 4, 2018

Can we use underscore char as the first character of a name?
Does puppet accept define _one::two ...?

@masatake
Copy link
Member

masatake commented Oct 4, 2018

The line in optlib/puppetManifest.ctags:

--_mtable-regex-PuppetManifest=defineStart/([a-zA-Z:][a-zA-Z0-9:]*)[ \n\t]*\(/\1/d/{tenter=skipArgs,blockHeadPopAtLast}{scope=push}

is wrong.
I wonder I should fix it with

--_mtable-regex-PuppetManifest=defineStart/([a-zA-Z:][_a-zA-Z0-9:]*)[ \n\t]*\(/\1/d/{tenter=skipArgs,blockHeadPopAtLast}{scope=push}

or
I wonder I should fix it with

--_mtable-regex-PuppetManifest=defineStart/([_a-zA-Z:][_a-zA-Z0-9:]*)[ \n\t]*\(/\1/d/{tenter=skipArgs,blockHeadPopAtLast}{scope=push}

.

masatake added a commit to masatake/ctags that referenced this issue Oct 4, 2018
@ahakanbaba
Copy link
Contributor Author

Can we use underscore char as the first character of a name?
Does puppet accept define _one::two ...?

Good question. I think only lowercase letters are allowed as the first character.
Looking at the docs, the regular expression to match is listed there as

\A[a-z][a-z0-9_]*\Z

The text at the docs also point this out.

The names of classes and defined resource types [...] must begin with a lowercase letter [.]

@masatake
Copy link
Member

masatake commented Oct 4, 2018

How about hyphen?
As far as reading https://puppet.com/docs/puppet/6.0/lang_data_string.html#bare-words, a hyphen is acceptable as part of the body in a name.

@ahakanbaba
Copy link
Contributor Author

Himm, I do not think bare-word is what follows a class or a define statement.
The docs here mention that:

The names of classes and defined resource types can consist of one or more namespace segments

If a "bare-word" were acceptable, I think the documentation at least would mention "bare-word"

I tried running puppet-lint as well

For the input.pp with a hyphen:

define one_o-ne::two($ensure) {
    file { "/tmp/fqdefinition": ensure => $ensure }
}

Prints an error about the hyphen in the name:

$ puppet-lint  input.pp
ERROR: one_o-ne::two not in autoload module layout on line 1
ERROR: defined type name containing a dash on line 1

Without the hypen charater:

define one_one::two($ensure) {
    file { "/tmp/fqdefinition": ensure => $ensure }
}

Prints only one error :

$ puppet-lint  input.pp
ERROR: one_one::two not in autoload module layout on line 1

@iankronquist
Copy link

Saw this totally by accident, I worked on the puppet parser a couple years ago for the summer. Unfortunately I can't remember any details about the language, but I believe that Heinrich Lindberg maintains the current puppet parser and should have definitive answers to any puppet parser related questions:
https://github.com/hlindberg
The canonical parser lives somewhere in here:
https://github.com/puppetlabs/puppet/tree/master/lib/puppet/parser

ahakanbaba added a commit to ahakanbaba/ctags that referenced this issue Oct 5, 2018
fixes universal-ctags#1901
According to puppet docs: https://puppet.com/docs/puppet/5.3/lang_reserved.html#classes-and-defined-resource-types
calss or a definition name can contain an underscore anywhere except the
first char.
Also added tests.
A minor name change for a test directory
ahakanbaba added a commit to ahakanbaba/ctags that referenced this issue Oct 6, 2018
According to puppet docs: https://puppet.com/docs/puppet/5.3/lang_reserved.html#classes-and-defined-resource-types
calss or a definition name can contain an underscore anywhere except the
first char.
Also added tests.
A minor name change for a test directory
fixes universal-ctags#1901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants