Skip to content

Commit b7ee7d3

Browse files
adamretteradamretter
committed
[refactor] Variables used by Binding Expressions should be EQName according to the XQuery spec; these can be processed at parse time
1 parent ef6c4ea commit b7ee7d3

File tree

9 files changed

+61
-49
lines changed

9 files changed

+61
-49
lines changed

‎exist-core/src/main/antlr/org/exist/xquery/parser/XQueryTree.g‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,9 @@ options {
128128

129129
private static class ForLetClause {
130130
XQueryAST ast;
131-
String varName;
131+
QName varName;
132132
SequenceType sequenceType= null;
133-
String posVar= null;
133+
QName posVar = null;
134134
Expression inputSequence;
135135
Expression action;
136136
FLWORClause.ClauseType type = FLWORClause.ClauseType.FOR;
@@ -1398,7 +1398,11 @@ throws PermissionDeniedException, EXistException, XPathException
13981398
)?
13991399
step=expr[inputSequence]
14001400
{
1401-
clause.varName= someVarName.getText();
1401+
try {
1402+
clause.varName = QName.parse(staticContext, someVarName.getText(), null);
1403+
} catch (final IllegalQNameException iqe) {
1404+
throw new XPathException(someVarName.getLine(), someVarName.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + someVarName.getText());
1405+
}
14021406
clause.inputSequence= inputSequence;
14031407
clauses.add(clause);
14041408
}
@@ -1449,7 +1453,11 @@ throws PermissionDeniedException, EXistException, XPathException
14491453
)?
14501454
step=expr[inputSequence]
14511455
{
1452-
clause.varName= everyVarName.getText();
1456+
try {
1457+
clause.varName = QName.parse(staticContext, everyVarName.getText(), null);
1458+
} catch (final IllegalQNameException iqe) {
1459+
throw new XPathException(everyVarName.getLine(), everyVarName.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + everyVarName.getText());
1460+
}
14531461
clause.inputSequence= inputSequence;
14541462
clauses.add(clause);
14551463
}
@@ -1585,11 +1593,21 @@ throws PermissionDeniedException, EXistException, XPathException
15851593
)?
15861594
(
15871595
posVar:POSITIONAL_VAR
1588-
{ clause.posVar= posVar.getText(); }
1596+
{
1597+
try {
1598+
clause.posVar = QName.parse(staticContext, posVar.getText(), null);
1599+
} catch (final IllegalQNameException iqe) {
1600+
throw new XPathException(posVar.getLine(), posVar.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + posVar.getText());
1601+
}
1602+
}
15891603
)?
15901604
step=expr [inputSequence]
15911605
{
1592-
clause.varName= varName.getText();
1606+
try {
1607+
clause.varName = QName.parse(staticContext, varName.getText(), null);
1608+
} catch (final IllegalQNameException iqe) {
1609+
throw new XPathException(varName.getLine(), varName.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + varName.getText());
1610+
}
15931611
clause.inputSequence= inputSequence;
15941612
clauses.add(clause);
15951613
}
@@ -1618,7 +1636,11 @@ throws PermissionDeniedException, EXistException, XPathException
16181636
)?
16191637
step=expr [inputSequence]
16201638
{
1621-
clause.varName= letVarName.getText();
1639+
try {
1640+
clause.varName = QName.parse(staticContext, letVarName.getText(), null);
1641+
} catch (final IllegalQNameException iqe) {
1642+
throw new XPathException(letVarName.getLine(), letVarName.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + letVarName.getText());
1643+
}
16221644
clause.inputSequence= inputSequence;
16231645
clauses.add(clause);
16241646
}
@@ -1759,7 +1781,11 @@ throws PermissionDeniedException, EXistException, XPathException
17591781
{
17601782
ForLetClause clause = new ForLetClause();
17611783
clause.ast = co;
1762-
clause.varName = countVarName.getText();
1784+
try {
1785+
clause.varName = QName.parse(staticContext, countVarName.getText(), null);
1786+
} catch (final IllegalQNameException iqe) {
1787+
throw new XPathException(countVarName.getLine(), countVarName.getColumn(), ErrorCodes.XPST0081, "No namespace defined for prefix " + countVarName.getText());
1788+
}
17631789
clause.type = FLWORClause.ClauseType.COUNT;
17641790
clause.inputSequence = null;
17651791
clauses.add(clause);

‎exist-core/src/main/java/org/exist/xquery/AbstractFLWORClause.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,10 @@ public AbstractFLWORClause(XQueryContext context) {
4141
}
4242

4343
@Override
44-
public LocalVariable createVariable(final String name) throws XPathException {
45-
try {
46-
final LocalVariable var = new LocalVariable(QName.parse(context, name, null));
47-
firstVar = var;
48-
return var;
49-
} catch (final IllegalQNameException e) {
50-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix " + name);
51-
}
44+
public LocalVariable createVariable(final QName name) throws XPathException {
45+
final LocalVariable var = new LocalVariable(name);
46+
firstVar = var;
47+
return var;
5248
}
5349

5450
@Override

‎exist-core/src/main/java/org/exist/xquery/BindingExpression.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import org.apache.logging.log4j.LogManager;
2525
import org.apache.logging.log4j.Logger;
26+
import org.exist.dom.QName;
2627
import org.exist.dom.persistent.*;
2728
import org.exist.numbering.NodeId;
2829
import org.exist.storage.UpdateListener;
@@ -41,7 +42,7 @@ public abstract class BindingExpression extends AbstractFLWORClause implements R
4142
protected final static SequenceType POSITIONAL_VAR_TYPE =
4243
new SequenceType(Type.INTEGER, Cardinality.EXACTLY_ONE);
4344

44-
protected String varName;
45+
protected QName varName;
4546
protected SequenceType sequenceType = null;
4647
protected Expression inputSequence;
4748

@@ -52,11 +53,11 @@ public BindingExpression(XQueryContext context) {
5253
super(context);
5354
}
5455

55-
public void setVariable(String qname) {
56-
varName = qname;
56+
public void setVariable(final QName varName) {
57+
this.varName = varName;
5758
}
5859

59-
public String getVariable() {
60+
public QName getVariable() {
6061
return this.varName;
6162
}
6263

‎exist-core/src/main/java/org/exist/xquery/CountClause.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*/
2222
package org.exist.xquery;
2323

24+
import org.exist.dom.QName;
2425
import org.exist.xquery.util.ExpressionDumper;
2526
import org.exist.xquery.value.Item;
2627
import org.exist.xquery.value.Sequence;
@@ -33,19 +34,19 @@
3334
*/
3435
public class CountClause extends AbstractFLWORClause {
3536

36-
final String varName;
37+
final QName varName;
3738

38-
public CountClause(final XQueryContext context, final String countName) {
39+
public CountClause(final XQueryContext context, final QName varName) {
3940
super(context);
40-
this.varName = countName;
41+
this.varName = varName;
4142
}
4243

4344
@Override
4445
public ClauseType getType() {
4546
return ClauseType.COUNT;
4647
}
4748

48-
public String getVarName() {
49+
public QName getVarName() {
4950
return varName;
5051
}
5152

‎exist-core/src/main/java/org/exist/xquery/FLWORClause.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*/
2222
package org.exist.xquery;
2323

24+
import org.exist.dom.QName;
2425
import org.exist.xquery.value.Sequence;
2526

2627
/**
@@ -102,7 +103,7 @@ enum ClauseType {
102103
* @return a new local variable, registered in the context
103104
* @throws XPathException if an error occurs whilst creating the variable
104105
*/
105-
LocalVariable createVariable(String name) throws XPathException;
106+
LocalVariable createVariable(QName name) throws XPathException;
106107

107108
/**
108109
* Returns the first variable created by this FLWOR clause for reference

‎exist-core/src/main/java/org/exist/xquery/ForExpr.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
*/
3434
public class ForExpr extends BindingExpression {
3535

36-
private String positionalVariable = null;
36+
private QName positionalVariable = null;
3737
private boolean allowEmpty = false;
3838
private boolean isOuterFor = true;
3939

@@ -53,7 +53,7 @@ public ClauseType getType() {
5353
*
5454
* @param var the name of the variable to set
5555
*/
56-
public void setPositionalVariable(String var) {
56+
public void setPositionalVariable(final QName var) {
5757
positionalVariable = var;
5858
}
5959

@@ -69,7 +69,7 @@ public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
6969
final AnalyzeContextInfo varContextInfo = new AnalyzeContextInfo(contextInfo);
7070
inputSequence.analyze(varContextInfo);
7171
// Declare the iteration variable
72-
final LocalVariable inVar = new LocalVariable(QName.parse(context, varName, null));
72+
final LocalVariable inVar = new LocalVariable(varName);
7373
inVar.setSequenceType(sequenceType);
7474
inVar.setStaticType(varContextInfo.getStaticReturnType());
7575
context.declareVariableBinding(inVar);
@@ -80,7 +80,7 @@ public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
8080
throw new XPathException(this, ErrorCodes.XQST0089,
8181
"bound variable and positional variable have the same name");
8282
}
83-
final LocalVariable posVar = new LocalVariable(QName.parse(context, positionalVariable, null));
83+
final LocalVariable posVar = new LocalVariable(positionalVariable);
8484
posVar.setSequenceType(POSITIONAL_VAR_TYPE);
8585
posVar.setStaticType(Type.INTEGER);
8686
context.declareVariableBinding(posVar);
@@ -89,8 +89,6 @@ public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
8989
final AnalyzeContextInfo newContextInfo = new AnalyzeContextInfo(contextInfo);
9090
newContextInfo.addFlag(SINGLE_STEP_EXECUTION);
9191
returnExpr.analyze(newContextInfo);
92-
} catch (final QName.IllegalQNameException e) {
93-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix");
9492
} finally {
9593
// restore the local variable stack
9694
context.popLocalVariables(mark);
@@ -135,7 +133,7 @@ public Sequence eval(Sequence contextSequence, Item contextItem)
135133
// Declare positional variable
136134
LocalVariable at = null;
137135
if (positionalVariable != null) {
138-
at = new LocalVariable(QName.parse(context, positionalVariable, null));
136+
at = new LocalVariable(positionalVariable);
139137
at.setSequenceType(POSITIONAL_VAR_TYPE);
140138
context.declareVariableBinding(at);
141139
}
@@ -187,8 +185,6 @@ public Sequence eval(Sequence contextSequence, Item contextItem)
187185
processItem(var, i.nextItem(), in, resultSequence, at, p);
188186
}
189187
}
190-
} catch (final QName.IllegalQNameException e) {
191-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix " + positionalVariable);
192188
} finally {
193189
// restore the local variable stack
194190
context.popLocalVariables(mark, resultSequence);

‎exist-core/src/main/java/org/exist/xquery/LetExpr.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,14 @@ public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException
5353
final AnalyzeContextInfo varContextInfo = new AnalyzeContextInfo(contextInfo);
5454
inputSequence.analyze(varContextInfo);
5555
//Declare the iteration variable
56-
final LocalVariable inVar = new LocalVariable(QName.parse(context, varName, null));
56+
final LocalVariable inVar = new LocalVariable(varName);
5757
inVar.setSequenceType(sequenceType);
5858
inVar.setStaticType(varContextInfo.getStaticReturnType());
5959
context.declareVariableBinding(inVar);
6060
//Reset the context position
6161
context.setContextSequencePosition(0, null);
6262

6363
returnExpr.analyze(contextInfo);
64-
} catch (final QName.IllegalQNameException e) {
65-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix " + varName);
6664
} finally {
6765
// restore the local variable stack
6866
context.popLocalVariables(mark);

‎exist-core/src/main/java/org/exist/xquery/QuantifiedExpression.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
*/
2222
package org.exist.xquery;
2323

24-
import org.exist.dom.QName;
2524
import org.exist.xquery.util.ExpressionDumper;
2625
import org.exist.xquery.value.BooleanValue;
2726
import org.exist.xquery.value.Item;
@@ -63,13 +62,11 @@ public ClauseType getType() {
6362
public void analyze(AnalyzeContextInfo contextInfo) throws XPathException {
6463
final LocalVariable mark = context.markLocalVariables(false);
6564
try {
66-
context.declareVariableBinding(new LocalVariable(QName.parse(context, varName, null)));
65+
context.declareVariableBinding(new LocalVariable(varName));
6766

6867
contextInfo.setParent(this);
6968
inputSequence.analyze(contextInfo);
7069
returnExpr.analyze(contextInfo);
71-
} catch (final QName.IllegalQNameException e) {
72-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix " + varName);
7370
} finally {
7471
context.popLocalVariables(mark);
7572
}
@@ -87,12 +84,7 @@ public Sequence eval(Sequence contextSequence, Item contextItem)
8784
{context.getProfiler().message(this, Profiler.START_SEQUENCES, "CONTEXT ITEM", contextItem.toSequence());}
8885
}
8986

90-
final LocalVariable var;
91-
try {
92-
var = new LocalVariable(QName.parse(context, varName, null));
93-
} catch (final QName.IllegalQNameException e) {
94-
throw new XPathException(this, ErrorCodes.XPST0081, "No namespace defined for prefix " + varName);
95-
}
87+
final LocalVariable var = new LocalVariable(varName);
9688

9789
final Sequence inSeq = inputSequence.eval(contextSequence, contextItem);
9890
if (sequenceType != null) {

‎exist-core/src/test/java/org/exist/xquery/CountExpressionTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import antlr.RecognitionException;
2525
import antlr.TokenStreamException;
26+
import org.exist.dom.QName;
2627
import org.exist.xquery.parser.XQueryAST;
2728
import org.exist.xquery.parser.XQueryLexer;
2829
import org.exist.xquery.parser.XQueryParser;
@@ -42,7 +43,7 @@
4243
public class CountExpressionTest {
4344

4445
@Test
45-
public void countTest() throws RecognitionException, XPathException, TokenStreamException {
46+
public void countTest() throws RecognitionException, XPathException, TokenStreamException, QName.IllegalQNameException {
4647
final String query = "xquery version \"3.1\";\n" +
4748
"for $p in $products\n" +
4849
"order by $p/sales descending\n" +
@@ -79,6 +80,6 @@ public void countTest() throws RecognitionException, XPathException, TokenStream
7980
assertEquals(XQueryParser.VARIABLE_BINDING, ast.getNextSibling().getFirstChild().getNextSibling().getNextSibling().getFirstChild().getType());
8081
assertTrue(((ForExpr)expr.getFirst()).returnExpr instanceof OrderByClause);
8182
assertTrue(((OrderByClause)(((ForExpr)expr.getFirst()).returnExpr)).returnExpr instanceof CountClause);
82-
assertEquals("rank", ((CountClause)((OrderByClause)(((ForExpr)expr.getFirst()).returnExpr)).returnExpr).varName);
83+
assertEquals(new QName("rank"), ((CountClause)((OrderByClause)(((ForExpr)expr.getFirst()).returnExpr)).returnExpr).varName);
8384
}
8485
}

0 commit comments

Comments
 (0)