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.

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

SONARPHP-1562 Add basic support for PropertyHooks #1302

Merged
merged 5 commits into from
Nov 7, 2024
Merged

SONARPHP-1562 Add basic support for PropertyHooks #1302

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49d4642
SONARPHP-1562 Add basic support for PropertyHooks
jonas-wielage-sonarsource Nov 7, 2024
8081c42
SonarQube issues
jonas-wielage-sonarsource Nov 7, 2024
616bf49
Additional testing
jonas-wielage-sonarsource Nov 7, 2024
6501c51
PR review
jonas-wielage-sonarsource Nov 7, 2024
36fadf3
GET / SET in grammar
jonas-wielage-sonarsource Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 43 additions & 10 deletions php-frontend/src/main/java/org/sonar/php/parser/PHPGrammar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import org.sonar.plugins.php.api.tree.declaration.NamespaceNameTree;
import org.sonar.plugins.php.api.tree.declaration.ParameterListTree;
import org.sonar.plugins.php.api.tree.declaration.ParameterTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookListTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookTree;
import org.sonar.plugins.php.api.tree.declaration.ReturnTypeClauseTree;
import org.sonar.plugins.php.api.tree.declaration.TypeNameTree;
import org.sonar.plugins.php.api.tree.declaration.TypeTree;
Expand Down Expand Up @@ -342,7 +344,7 @@ public EnumDeclarationTree ENUM_DECLARATION() {
}

/**
* In contrast to class declarations, enums cannot contain properties. They do allow enum cases as an addition.
* In contrast to class declarations, enums cannot contain properties. They do allow enum cases as an addition.
*/
public ClassMemberTree ENUM_MEMBER() {
return b.<ClassMemberTree>nonterminal(PHPLexicalGrammar.ENUM_MEMBER).is(
Expand Down Expand Up @@ -410,15 +412,24 @@ public ConstantDeclarationTree CONSTANT_DECLARATION() {

public ClassPropertyDeclarationTree CLASS_VARIABLE_DECLARATION() {
return b.<ClassPropertyDeclarationTree>nonterminal(PHPLexicalGrammar.CLASS_VARIABLE_DECLARATION).is(
f.classVariableDeclaration(
b.zeroOrMore(ATTRIBUTE_GROUP()),
b.firstOf(
f.singleToken(b.token(PHPKeyword.VAR)),
b.oneOrMore(MEMBER_MODIFIER())),
b.optional(DECLARED_TYPE()),
VARIABLE_DECLARATION(),
b.zeroOrMore(f.newTuple(b.token(COMMA), VARIABLE_DECLARATION())),
EOS()));
b.firstOf(
f.classVariableDeclaration(
b.zeroOrMore(ATTRIBUTE_GROUP()),
b.firstOf(
f.singleToken(b.token(PHPKeyword.VAR)),
b.oneOrMore(MEMBER_MODIFIER())),
b.optional(DECLARED_TYPE()),
VARIABLE_DECLARATION(),
PROPERTY_HOOK_LIST()),
f.classVariableDeclaration(
b.zeroOrMore(ATTRIBUTE_GROUP()),
b.firstOf(
f.singleToken(b.token(PHPKeyword.VAR)),
b.oneOrMore(MEMBER_MODIFIER())),
b.optional(DECLARED_TYPE()),
VARIABLE_DECLARATION(),
b.zeroOrMore(f.newTuple(b.token(COMMA), VARIABLE_DECLARATION())),
EOS())));
}

public SyntaxToken VISIBILITY_MODIFIER() {
Expand Down Expand Up @@ -636,6 +647,28 @@ public DeclaredTypeTree DECLARED_TYPE() {
b.firstOf(DNF_TYPE(), UNION_TYPE(), INTERSECTION_TYPE(), TYPE()));
}

public PropertyHookListTree PROPERTY_HOOK_LIST() {
return b.<PropertyHookListTree>nonterminal(PHPLexicalGrammar.PROPERTY_HOOK_LIST).is(
f.propertyHookList(
b.token(LCURLYBRACE),
b.oneOrMore(PROPERTY_HOOK()),
b.token(RCURLYBRACE)));
}

public PropertyHookTree PROPERTY_HOOK() {
return b.<PropertyHookTree>nonterminal(PHPLexicalGrammar.PROPERTY_HOOK).is(
f.propertyHook(
b.zeroOrMore(ATTRIBUTE_GROUP()),
b.zeroOrMore(MEMBER_MODIFIER()),
jonas-wielage-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
b.optional(b.token(PHPPunctuator.AMPERSAND)),
NAME_IDENTIFIER_OR_KEYWORD(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of allowing any string here, couldn't we explicitly only allow get or set, with possibly enum values in the AST?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to do it by adding it to PHPKeywordsbut this will conflict the parser when other method declarations try to declare getor setmethods.
If you know another way, I'm happy to change it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one!

b.optional(PARAMETER_LIST()),
b.optional(b.token(DOUBLEARROW)),
b.firstOf(
EOS(),
INNER_STATEMENT())));
jonas-wielage-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* [ END ] Declaration
*/
Expand Down
3 changes: 3 additions & 0 deletions php-frontend/src/main/java/org/sonar/php/parser/PHPLexicalGrammar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public enum PHPLexicalGrammar implements GrammarRuleKey {
MEMBER_CONST_DECLARATION,
FUNCTION_CALL_ARGUMENT,

PROPERTY_HOOK_LIST,
PROPERTY_HOOK,

TRAIT_METHOD_REFERENCE_FULLY_QUALIFIED,
TRAIT_METHOD_REFERENCE,
TRAIT_ALIAS,
Expand Down
51 changes: 51 additions & 0 deletions php-frontend/src/main/java/org/sonar/php/parser/TreeFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
import org.sonar.php.tree.impl.declaration.NamespaceNameTreeImpl;
import org.sonar.php.tree.impl.declaration.ParameterListTreeImpl;
import org.sonar.php.tree.impl.declaration.ParameterTreeImpl;
import org.sonar.php.tree.impl.declaration.PropertyHookListTreeImpl;
import org.sonar.php.tree.impl.declaration.PropertyHookTreeImpl;
import org.sonar.php.tree.impl.declaration.ReturnTypeClauseTreeImpl;
import org.sonar.php.tree.impl.declaration.TraitAliasTreeImpl;
import org.sonar.php.tree.impl.declaration.TraitMethodReferenceTreeImpl;
Expand Down Expand Up @@ -162,6 +164,8 @@
import org.sonar.plugins.php.api.tree.declaration.NamespaceNameTree;
import org.sonar.plugins.php.api.tree.declaration.ParameterListTree;
import org.sonar.plugins.php.api.tree.declaration.ParameterTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookListTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookTree;
import org.sonar.plugins.php.api.tree.declaration.ReturnTypeClauseTree;
import org.sonar.plugins.php.api.tree.declaration.TypeNameTree;
import org.sonar.plugins.php.api.tree.declaration.TypeTree;
Expand Down Expand Up @@ -477,9 +481,24 @@ public ClassPropertyDeclarationTree classVariableDeclaration(
modifierTokens,
typeAnnotation.orNull(),
separatedList(firstVariable, additionalVariables),
null,
eosToken);
}

public ClassPropertyDeclarationTree classVariableDeclaration(
Optional<List<AttributeGroupTree>> attributes,
List<SyntaxToken> modifierTokens,
Optional<DeclaredTypeTree> typeAnnotation,
VariableDeclarationTree variable,
PropertyHookListTree propertyHookListTree) {
return ClassPropertyDeclarationTreeImpl.variable(attributes.or(Collections.emptyList()),
modifierTokens,
typeAnnotation.orNull(),
separatedList(variable, Optional.absent()),
propertyHookListTree,
null);
}

public MethodDeclarationTree methodDeclaration(
Optional<List<AttributeGroupTree>> attributes,
Optional<List<SyntaxToken>> modifiers,
Expand Down Expand Up @@ -681,6 +700,38 @@ public EnumCaseTree enumCase(Optional<List<AttributeGroupTree>> attributes, Synt
return new EnumCaseTreeImpl(attributes.or(Collections.emptyList()), caseToken, name, equalToken, value, eosToken);
}

public PropertyHookListTree propertyHookList(
InternalSyntaxToken openCurlyBrace,
List<PropertyHookTree> hooks,
InternalSyntaxToken closeCurlyBrace) {
return new PropertyHookListTreeImpl(openCurlyBrace, hooks, closeCurlyBrace);
}

public PropertyHookTree propertyHook(
Optional<List<AttributeGroupTree>> attributes,
Optional<List<SyntaxToken>> modifiers,
Optional<InternalSyntaxToken> referenceToken,
NameIdentifierTree name,
Optional<ParameterListTree> parameters,
Optional<InternalSyntaxToken> doubleArrowToken,
Tree body) {
throwOnUnrecognizedPropertyHookName(name);
return new PropertyHookTreeImpl(
attributes.or(Collections.emptyList()),
optionalList(modifiers),
referenceToken.orNull(),
name,
parameters.orNull(),
doubleArrowToken.orNull(),
body);
}

private static void throwOnUnrecognizedPropertyHookName(NameIdentifierTree name) {
if (!"get".equals(name.text()) && !"set".equals(name.text())) {
throw new RecognitionException(((PHPTree) name).getLine(), "Declared property hook must be named \"get\" or \"set\"");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to me to have part of the grammar logic in the TreeFactory class. As mentioned above I would strictly match get/set in the grammar and not grab any word.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but didn't find a good way to do it. I May try again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one!

}

/**
* [ END ] Declarations
*/
Expand Down
29 changes: 21 additions & 8 deletions ...d/src/main/java/org/sonar/php/tree/impl/declaration/ClassPropertyDeclarationTreeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.sonar.plugins.php.api.tree.declaration.AttributeGroupTree;
import org.sonar.plugins.php.api.tree.declaration.ClassPropertyDeclarationTree;
import org.sonar.plugins.php.api.tree.declaration.DeclaredTypeTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookListTree;
import org.sonar.plugins.php.api.tree.declaration.TypeTree;
import org.sonar.plugins.php.api.tree.declaration.UnionTypeTree;
import org.sonar.plugins.php.api.tree.declaration.VariableDeclarationTree;
Expand All @@ -44,6 +45,9 @@ public class ClassPropertyDeclarationTreeImpl extends PHPTree implements ClassPr
private final List<AttributeGroupTree> attributeGroups;
private final List<SyntaxToken> modifierTokens;
private final SeparatedListImpl<VariableDeclarationTree> declarations;
@Nullable
private final PropertyHookListTree propertyHooks;
@Nullable
private final InternalSyntaxToken eosToken;
@Nullable
private final DeclaredTypeTree typeAnnotation;
Expand All @@ -54,25 +58,29 @@ private ClassPropertyDeclarationTreeImpl(
List<SyntaxToken> modifierTokens,
@Nullable DeclaredTypeTree typeAnnotation,
SeparatedListImpl<VariableDeclarationTree> declarations,
InternalSyntaxToken eosToken) {
@Nullable PropertyHookListTree propertyHooks,
@Nullable InternalSyntaxToken eosToken) {
this.kind = kind;
this.attributeGroups = attributeGroups;
this.modifierTokens = modifierTokens;
this.typeAnnotation = typeAnnotation;
this.declarations = declarations;
this.propertyHooks = propertyHooks;
this.eosToken = eosToken;
}

public static ClassPropertyDeclarationTree variable(List<AttributeGroupTree> attributes,
List<SyntaxToken> modifierTokens,
@Nullable DeclaredTypeTree typeAnnotation,
SeparatedListImpl<VariableDeclarationTree> declarations,
InternalSyntaxToken eosToken) {
@Nullable PropertyHookListTree propertyHook,
jonas-wielage-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
@Nullable InternalSyntaxToken eosToken) {
return new ClassPropertyDeclarationTreeImpl(Kind.CLASS_PROPERTY_DECLARATION,
attributes,
Collections.unmodifiableList(modifierTokens),
typeAnnotation,
declarations,
propertyHook,
eosToken);
}

Expand All @@ -90,6 +98,7 @@ public static ClassPropertyDeclarationTree constant(List<AttributeGroupTree> att
Collections.unmodifiableList(modifierTokens),
typeAnnotation,
declarations,
null,
eosToken);
}

Expand Down Expand Up @@ -132,6 +141,13 @@ public SeparatedListImpl<VariableDeclarationTree> declarations() {
return declarations;
}

@Nullable
@Override
public PropertyHookListTree propertyHooks() {
return propertyHooks;
}

@Nullable
@Override
public SyntaxToken eosToken() {
return eosToken;
Expand Down Expand Up @@ -167,13 +183,10 @@ public Iterator<Tree> childrenIterator() {
return IteratorUtils.concat(
attributeGroups.iterator(),
modifierTokens.iterator(),
nullableIterator(typeAnnotation),
IteratorUtils.nullableIterator(typeAnnotation),
declarations.elementsAndSeparators(),
IteratorUtils.iteratorOf(eosToken));
}

private static Iterator<? extends Tree> nullableIterator(@Nullable Tree tree) {
return tree == null ? Collections.emptyIterator() : IteratorUtils.iteratorOf(tree);
IteratorUtils.nullableIterator(propertyHooks),
IteratorUtils.nullableIterator(eosToken));
}

@Override
Expand Down
82 changes: 82 additions & 0 deletions php-frontend/src/main/java/org/sonar/php/tree/impl/declaration/PropertyHookListTreeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* SonarQube PHP Plugin
* Copyright (C) 2010-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.php.tree.impl.declaration;

import java.util.Iterator;
import java.util.List;
import javax.annotation.Nullable;
import org.sonar.php.tree.impl.PHPTree;
import org.sonar.php.tree.impl.lexical.InternalSyntaxToken;
import org.sonar.php.utils.collections.IteratorUtils;
import org.sonar.plugins.php.api.tree.Tree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookListTree;
import org.sonar.plugins.php.api.tree.declaration.PropertyHookTree;
import org.sonar.plugins.php.api.visitors.VisitorCheck;

public class PropertyHookListTreeImpl extends PHPTree implements PropertyHookListTree {

private static final Kind KIND = Kind.PROPERTY_HOOK_LIST;

private final InternalSyntaxToken openCurlyBrace;
private final List<PropertyHookTree> hooks;
private final InternalSyntaxToken closeCurlyBrace;

public PropertyHookListTreeImpl(
InternalSyntaxToken openCurlyBrace, List<PropertyHookTree> hooks, InternalSyntaxToken closeCurlyBrace) {
this.openCurlyBrace = openCurlyBrace;
this.hooks = hooks;
this.closeCurlyBrace = closeCurlyBrace;
}

@Override
public List<PropertyHookTree> hooks() {
return hooks;
}

@Override
public Iterator<Tree> childrenIterator() {
return IteratorUtils.concat(
IteratorUtils.iteratorOf(openCurlyBrace),
hooks.iterator(),
IteratorUtils.iteratorOf(closeCurlyBrace));
}

@Override
public void accept(VisitorCheck visitor) {
visitor.visitPropertyHookList(this);
}

@Override
public Kind getKind() {
return KIND;
}

@Nullable
@Override
public InternalSyntaxToken openCurlyBrace() {
return openCurlyBrace;
}

@Nullable
@Override
public InternalSyntaxToken closeCurlyBrace() {
return closeCurlyBrace;
}
jonas-wielage-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
Loading