You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by natgabb <gi...@git.apache.org> on 2016/07/21 19:35:22 UTC

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

GitHub user natgabb opened a pull request:

    https://github.com/apache/groovy/pull/370

    Add access to declared variables in the VariableScope

    For easier parsing of symbols.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/natgabb/groovy add-declared-var-access

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/groovy/pull/370.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #370
    
----
commit 0f2c87303b6c5e9f9fb730091cacc80724bf6e07
Author: Natacha Gabbamonte <ng...@palantir.com>
Date:   2016-07-21T19:32:04Z

    Add access to declared variables in the VariableScope

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/groovy/pull/370


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by natgabb <gi...@git.apache.org>.
Github user natgabb commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/370#discussion_r71992968
  
    --- Diff: src/main/org/codehaus/groovy/ast/VariableScope.java ---
    @@ -178,4 +178,26 @@ public Object removeReferencedClassVariable(String name) {
         public Iterator<Variable> getReferencedClassVariablesIterator() {
             return getReferencedClassVariables().values().iterator();
         }
    -}
    \ No newline at end of file
    +
    +    /**
    +     * Gets a map containing the variables declared in this scope.
    +     * This map cannot be modified.
    +     * @return a map containing the declared variable references
    +     */
    +    public Map<String, Variable> getDeclaredVariables() {
    --- End diff --
    
    Actually `Collections.EMPTY_MAP` is an immutable map. See: https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#EMPTY_MAP
    This means the map will be immutable in both cases since `new HashMap<String, Integer>()` is not equal to the `Collections.EMPTY_MAP`. I've reset the last commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by natgabb <gi...@git.apache.org>.
Github user natgabb commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/370#discussion_r71991911
  
    --- Diff: src/main/org/codehaus/groovy/ast/VariableScope.java ---
    @@ -178,4 +178,26 @@ public Object removeReferencedClassVariable(String name) {
         public Iterator<Variable> getReferencedClassVariablesIterator() {
             return getReferencedClassVariables().values().iterator();
         }
    -}
    \ No newline at end of file
    +
    +    /**
    +     * Gets a map containing the variables declared in this scope.
    +     * This map cannot be modified.
    +     * @return a map containing the declared variable references
    +     */
    +    public Map<String, Variable> getDeclaredVariables() {
    --- End diff --
    
    Good point! I fixed it. This is also the case in `getReferencedClassVariables', would you like me to change it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by baybatu <gi...@git.apache.org>.
Github user baybatu commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/370#discussion_r71832411
  
    --- Diff: src/main/org/codehaus/groovy/ast/VariableScope.java ---
    @@ -178,4 +178,26 @@ public Object removeReferencedClassVariable(String name) {
         public Iterator<Variable> getReferencedClassVariablesIterator() {
             return getReferencedClassVariables().values().iterator();
         }
    -}
    \ No newline at end of file
    +
    +    /**
    +     * Gets a map containing the variables declared in this scope.
    +     * This map cannot be modified.
    +     * @return a map containing the declared variable references
    +     */
    +    public Map<String, Variable> getDeclaredVariables() {
    --- End diff --
    
    In javadoc, it says "This map cannot be modified" but this method can return two type of Map implementations: HashMap (see putDeclaredVariable method in the same file) and UnmodifiableMap stated in 'else' block.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by blackdrag <gi...@git.apache.org>.
Github user blackdrag commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/370#discussion_r72755079
  
    --- Diff: src/main/org/codehaus/groovy/ast/VariableScope.java ---
    @@ -178,4 +178,26 @@ public Object removeReferencedClassVariable(String name) {
         public Iterator<Variable> getReferencedClassVariablesIterator() {
             return getReferencedClassVariables().values().iterator();
         }
    -}
    \ No newline at end of file
    +
    +    /**
    +     * Gets a map containing the variables declared in this scope.
    +     * This map cannot be modified.
    +     * @return a map containing the declared variable references
    +     */
    +    public Map<String, Variable> getDeclaredVariables() {
    --- End diff --
    
    please do not change it for getReferencedClassVariables, we have code directly mutating that map


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] groovy pull request #370: Add access to declared variables in the VariableSc...

Posted by baybatu <gi...@git.apache.org>.
Github user baybatu commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/370#discussion_r71996755
  
    --- Diff: src/main/org/codehaus/groovy/ast/VariableScope.java ---
    @@ -178,4 +178,26 @@ public Object removeReferencedClassVariable(String name) {
         public Iterator<Variable> getReferencedClassVariablesIterator() {
             return getReferencedClassVariables().values().iterator();
         }
    -}
    \ No newline at end of file
    +
    +    /**
    +     * Gets a map containing the variables declared in this scope.
    +     * This map cannot be modified.
    +     * @return a map containing the declared variable references
    +     */
    +    public Map<String, Variable> getDeclaredVariables() {
    --- End diff --
    
    Yes, you're right :) \U0001f44d 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---