added support to easy discovering comments for statement (for suppression)

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java

@@ -28,74 +28,16 @@ import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 
-public class CaseBreakChecker extends AbstractIndexAstChecker implements
+public class CaseBreakChecker extends AbstractIndexAstChecker implements ICheckerWithPreferences {
-		ICheckerWithPreferences {
 	public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.CaseBreakProblem"; //$NON-NLS-1$
 	public static final String PARAM_LAST_CASE = "last_case_param"; //$NON-NLS-1$
 	public static final String PARAM_EMPTY_CASE = "empty_case_param"; //$NON-NLS-1$
 	public static final String PARAM_NO_BREAK_COMMENT = "no_break_comment"; //$NON-NLS-1$
 	public static final String DEFAULT_NO_BREAK_COMMENT = "no break"; //$NON-NLS-1$
-	private CommentsIterator _commentsIt; // Iterator over comments
 	private Boolean _checkLastCase; // Should we check the last case in the switch?
 	private Boolean _checkEmptyCase; // Should we check an empty case (a case without any statements within it)
 	private String _noBreakComment; // The comment suppressing this warning 
 
-	/**
-	 * This class receives the comments of the AST and iterates over them
-	 */
-	class CommentsIterator {
-		private int _next; // The next comment's index
-		private IASTComment[] _comments;
-
-		CommentsIterator(IASTComment[] comments) {
-			_comments = comments;
-			_next = 0;
-		}
-
-		/**
-		 * @return Is there an unvisited comment?
-		 */
-		public boolean hasNext() {
-			return (_next < _comments.length);
-		}
-
-		/**
-		 * @return The next comment (doesn't automatically advance to the next
-		 *         comment.
-		 *         i.e. Calling this function twice may return the same value).
-		 *         See {@link#advance}
-		 */
-		public IASTComment getNext() {
-			return (_comments[_next]);
-		}
-
-		/**
-		 * @param node The node to compare the comment's location to
-		 * @return Is the next comment located after 'node'
-		 */
-		public boolean isNextAfterThis(IASTNode node) {
-			return (_comments[_next].getFileLocation().getNodeOffset() > node
-					.getFileLocation().getNodeOffset());
-		}
-
-		/**
-		 * @param node The node to compare the comment's location to
-		 * @return Is the next comment located after 'node' ends
-		 */
-		public boolean isNextAfterThisEnds(IASTNode node) {
-			return (_comments[_next].getFileLocation().getNodeOffset() > node
-					.getFileLocation().getNodeOffset()
-					+ node.getFileLocation().getNodeLength());
-		}
-
-		/**
-		 * Advance to the next comment
-		 */
-		public void advance() {
-			_next++;
-		}
-	}
-
 	/**
 	 * This visitor looks for "switch" statements and invokes "SwitchVisitor" on
 	 * them
@@ -111,43 +53,41 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 		/**
 		 * @param statement
 		 * @return true iff the statement is directly under the "switch" and not
-		 *  in the scope of some loop statement, such as "while". 
+		 *         in the scope of some loop statement, such as "while".
 		 */
 		private boolean doesStatementAffectThisSwitch(IASTStatement statement) {
-			IASTNode parent = statement.getParent(); 
+			IASTNode parent = statement.getParent();
-			if(parent == _switchStatement)
+			if (parent == _switchStatement)
 				return true;
-			if(parent instanceof IASTCompoundStatement)
+			if (parent instanceof IASTCompoundStatement)
-				return doesStatementAffectThisSwitch((IASTCompoundStatement)parent);
+				return doesStatementAffectThisSwitch((IASTCompoundStatement) parent);
-			
 			return false;
 		}
-		
+
 		/**
 		 * @param statement
 		 * @return true iff the statement is on of:
-		 * - "break" (checks that the break actually exists the "switch")
+		 *         - "break" (checks that the break actually exists the
-		 * - "return"
+		 *         "switch")
-		 * - "continue"
+		 *         - "return"
-		 * - "goto" (does not check that the goto actually exists the switch)
+		 *         - "continue"
-		 * - "thorw"
+		 *         - "goto" (does not check that the goto actually exists the
-		 * - "exit"
+		 *         switch)
+		 *         - "thorw"
+		 *         - "exit"
 		 */
 		protected boolean isBreakOrExitStatement(IASTStatement statement) {
 			CxxAstUtils utils = CxxAstUtils.getInstance();
 			return (statement instanceof IASTBreakStatement && doesStatementAffectThisSwitch(statement))
-					|| statement instanceof IASTReturnStatement
+					|| statement instanceof IASTReturnStatement || statement instanceof IASTContinueStatement
-					|| statement instanceof IASTContinueStatement
+					|| statement instanceof IASTGotoStatement || utils.isThrowStatement(statement) || utils.isExitStatement(statement);
-					|| statement instanceof IASTGotoStatement
+		}
-					|| utils.isThrowStatement(statement) || utils.isExitStatement(statement);
+
-		}		
-		
 		@Override
 		public int visit(IASTStatement statement) {
 			if (statement instanceof IASTSwitchStatement) {
 				// Are we already visiting this statement?
-				if (_switchStatement == null
+				if (_switchStatement == null || !statement.equals(_switchStatement)) {
-						|| !statement.equals(_switchStatement)) {
 					SwitchVisitor switch_visitor = new SwitchVisitor(statement);
 					statement.accept(switch_visitor);
 					return PROCESS_SKIP;
@@ -191,8 +131,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 		 * @return Was a "break" statement the last statement in this case
 		 */
 		private boolean breakFoundPrevious() {
-			return _prev_normal_stmnt_offset < _prev_break_stmnt_offset
+			return _prev_normal_stmnt_offset < _prev_break_stmnt_offset && _prev_case_stmnt_offset < _prev_break_stmnt_offset;
-					&& _prev_case_stmnt_offset < _prev_break_stmnt_offset;
 		}
 
 		/**
@@ -232,8 +171,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 
 		@Override
 		public int visit(IASTStatement statement) {
-			if (statement instanceof IASTCaseStatement
+			if (statement instanceof IASTCaseStatement || statement instanceof IASTDefaultStatement) {
-					|| statement instanceof IASTDefaultStatement) {
 				if (_first_case_statement) {
 					/*
 					 * This is the first "case", i.e. the beginning of the
@@ -246,13 +184,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 					 * has just ended,
 					 * Let's check that case and see how it ended...
 					 */
-					IASTComment comment = null;
+					IASTComment comment = getLeadingComment(statement);
-					// Do we have a comment which is before this "case" statement (but after the previous statement)?
-					while (_commentsIt.hasNext()
-							&& !_commentsIt.isNextAfterThis(statement)) {
-						comment = _commentsIt.getNext();
-						_commentsIt.advance();
-					}
 					/*
 					 * 'comment' is the last comment found in this case (after
 					 * the last statement in this "case"
@@ -260,40 +192,44 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 					checkPreviousCase(comment, false);
 				}
 				/* Update variables with the new opened "case" */
-				_prev_case_stmnt_offset = statement.getFileLocation()
+				_prev_case_stmnt_offset = statement.getFileLocation().getNodeOffset();
-						.getNodeOffset();
 				_prev_case_stmnt = statement;
 			} else if (isBreakOrExitStatement(statement)) { // A relevant "break" statement
-				_prev_break_stmnt_offset = statement.getFileLocation()
+				_prev_break_stmnt_offset = statement.getFileLocation().getNodeOffset();
-						.getNodeOffset();
 			} else { // a non-switch related statement
-				_prev_normal_stmnt_offset = statement.getFileLocation()
+				_prev_normal_stmnt_offset = statement.getFileLocation().getNodeOffset();
-						.getNodeOffset();
 			}
-			/* advance comments we already passed */
-			while (_commentsIt.hasNext()
-					&& !_commentsIt.isNextAfterThis(statement))
-				_commentsIt.advance();
 			return super.visit(statement); // This would handle nested "switch"s
 		}
 
+		/**
+		 * @param statement
+		 * @return
+		 */
+		public IASTComment getLeadingComment(IASTStatement statement) {
+			return CxxAstUtils.getInstance().getLeadingComment(statement);
+		}
+
+
+		/**
+		 * @param statement
+		 * @return
+		 */
+		public IASTComment getFreestandingComment(IASTStatement statement) {
+			return CxxAstUtils.getInstance().getFreestandingComment(statement);
+		}
+
 		@Override
 		public int leave(IASTStatement statement) {
 			/*
 			 * Are we leaving the "switch" altogether? (we need to see how the
 			 * last "case" ended)
 			 */
-			if (_checkLastCase && statement instanceof IASTCompoundStatement
+			if (_checkLastCase && statement instanceof IASTCompoundStatement && statement.getParent() == _switchStatement) {
-					&& statement.getParent() == _switchStatement) {
+				IASTComment comment = getFreestandingComment(statement);
-				IASTComment comment = null;
-				// is "Next" still in the switch's scope? if it is it was after the last statement
-				while (_commentsIt.hasNext()
-						&& !_commentsIt.isNextAfterThisEnds(statement)) {
-					comment = _commentsIt.getNext();
-					_commentsIt.advance();
-				}
 				/*
-				 * 'comment' is the last comment found in this case (after the
+				 * 'comment' is the last comment found in this case (after
+				 * the
 				 * last statement in this "case"
 				 */
 				checkPreviousCase(comment, true);
@@ -310,29 +246,17 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements
 
 	public void initPreferences(IProblemWorkingCopy problem) {
 		super.initPreferences(problem);
-		addPreference(
+		addPreference(problem, PARAM_NO_BREAK_COMMENT, CheckersMessages.CaseBreakChecker_DefaultNoBreakCommentDescription,
-				problem,
-				PARAM_NO_BREAK_COMMENT,
-				CheckersMessages.CaseBreakChecker_DefaultNoBreakCommentDescription,
 				DEFAULT_NO_BREAK_COMMENT);
-		addPreference(problem, PARAM_LAST_CASE,
+		addPreference(problem, PARAM_LAST_CASE, CheckersMessages.CaseBreakChecker_LastCaseDescription, Boolean.TRUE);
-				CheckersMessages.CaseBreakChecker_LastCaseDescription,
+		addPreference(problem, PARAM_EMPTY_CASE, CheckersMessages.CaseBreakChecker_EmptyCaseDescription, Boolean.FALSE);
-				Boolean.TRUE);
-		addPreference(problem, PARAM_EMPTY_CASE,
-				CheckersMessages.CaseBreakChecker_EmptyCaseDescription,
-				Boolean.FALSE);
-
 	}
 
 	public void processAst(IASTTranslationUnit ast) {
-		_checkLastCase = (Boolean) getPreference(
+		_checkLastCase = (Boolean) getPreference(getProblemById(ER_ID, getFile()), PARAM_LAST_CASE);
-				getProblemById(ER_ID, getFile()), PARAM_LAST_CASE);
+		_checkEmptyCase = (Boolean) getPreference(getProblemById(ER_ID, getFile()), PARAM_EMPTY_CASE);
-		_checkEmptyCase = (Boolean) getPreference(
+		_noBreakComment = (String) getPreference(getProblemById(ER_ID, getFile()), PARAM_NO_BREAK_COMMENT);
-				getProblemById(ER_ID, getFile()), PARAM_EMPTY_CASE);
-		_noBreakComment = (String) getPreference(
-				getProblemById(ER_ID, getFile()), PARAM_NO_BREAK_COMMENT);
 		SwitchFindingVisitor visitor = new SwitchFindingVisitor();
-		_commentsIt = new CommentsIterator(ast.getComments());
 		ast.accept(visitor);
 	}
 }

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/CxxAstUtils.java

@@ -14,10 +14,14 @@ package org.eclipse.cdt.codan.core.cxx;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
+import org.eclipse.cdt.codan.core.cxx.model.CxxModelsCache;
+import org.eclipse.cdt.codan.core.cxx.model.ICodanCommentMap;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
+import org.eclipse.cdt.core.dom.ast.IASTComment;
 import org.eclipse.cdt.core.dom.ast.IASTCompositeTypeSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
@@ -390,4 +394,52 @@ public final class CxxAstUtils {
 		IASTExpression functionNameExpression = ((IASTFunctionCallExpression) expression).getFunctionNameExpression();
 		return functionNameExpression.getRawSignature().equals("exit"); //$NON-NLS-1$
 	}
+	
+	/**
+	 * @param statement
+	 * @return
+	 */
+	public IASTComment getLeadingComment(IASTStatement statement) {
+		IASTComment comment = null;
+		ICodanCommentMap map = CxxModelsCache.getInstance().getCommentedNodeMap(statement.getTranslationUnit());
+		if (map != null) {
+			List<IASTComment> comms = map.getLeadingCommentsForNode(statement);
+			if (comms.size() > 0) {
+				comment = comms.get(comms.size() - 1);
+			}
+		}
+		return comment;
+	}
+
+	/**
+	 * @param statement
+	 * @return
+	 */
+	public IASTComment getTrailingComment(IASTStatement statement) {
+		IASTComment comment = null;
+		ICodanCommentMap map = CxxModelsCache.getInstance().getCommentedNodeMap(statement.getTranslationUnit());
+		if (map != null) {
+			List<IASTComment> comms = map.getTrailingCommentsForNode(statement);
+			if (comms.size() > 0) {
+				comment = comms.get(0);
+			}
+		}
+		return comment;
+	}
+
+	/**
+	 * @param statement
+	 * @return
+	 */
+	public IASTComment getFreestandingComment(IASTStatement statement) {
+		IASTComment comment = null;
+		ICodanCommentMap map = CxxModelsCache.getInstance().getCommentedNodeMap(statement.getTranslationUnit());
+		if (map != null) {
+			List<IASTComment> comms = map.getFreestandingForNode(statement);
+			if (comms.size() > 0) {
+				comment = comms.get(comms.size() - 1);
+			}
+		}
+		return comment;
+	}
 }

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/CodanCommentMap.java

@@ -0,0 +1,59 @@
+/*******************************************************************************
+ * Copyright (c) 2009,2010 QNX Software Systems
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    QNX Software Systems (Alena Laskavaia)  - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.core.cxx.internal.model;
+
+import java.util.List;
+
+import org.eclipse.cdt.codan.core.cxx.model.ICodanCommentMap;
+import org.eclipse.cdt.core.dom.ast.IASTComment;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap;
+
+/**
+ * Implementation of ICodanCommentMap. 
+ */
+public class CodanCommentMap implements ICodanCommentMap {
+	private NodeCommentMap commentedNodeMap;
+
+	/**
+	 * @param commentedNodeMap
+	 */
+	public CodanCommentMap(NodeCommentMap commentedNodeMap) {
+		this.commentedNodeMap = commentedNodeMap;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see org.eclipse.cdt.codan.core.cxx.model.ICodanCommentMap#
+	 * getTrailingCommentsForNode(org.eclipse.cdt.core.dom.ast.IASTNode)
+	 */
+	public List<IASTComment> getTrailingCommentsForNode(IASTNode node) {
+		return commentedNodeMap.getTrailingCommentsForNode(node);
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * 
+	 * @see org.eclipse.cdt.codan.core.cxx.model.ICodanCommentMap#
+	 * getLeadingCommentsForNode(org.eclipse.cdt.core.dom.ast.IASTNode)
+	 */
+	public List<IASTComment> getLeadingCommentsForNode(IASTNode node) {
+		return commentedNodeMap.getLeadingCommentsForNode(node);
+	}
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.cdt.codan.core.cxx.model.ICodanCommentMap#getFreestandingForNode(org.eclipse.cdt.core.dom.ast.IASTStatement)
+	 */
+	public List<IASTComment> getFreestandingForNode(IASTNode node) {
+		return commentedNodeMap.getFreestandingCommentsForNode(node);
+	}
+}

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java

@@ -12,6 +12,8 @@ package org.eclipse.cdt.codan.core.cxx.model;
 
 import java.util.WeakHashMap;
 
+import org.eclipse.cdt.codan.core.cxx.Activator;
+import org.eclipse.cdt.codan.core.cxx.internal.model.CodanCommentMap;
 import org.eclipse.cdt.codan.core.cxx.internal.model.cfg.CxxControlFlowGraph;
 import org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph;
 import org.eclipse.cdt.core.CCorePlugin;
@@ -21,6 +23,7 @@ import org.eclipse.cdt.core.index.IIndex;
 import org.eclipse.cdt.core.model.CoreModel;
 import org.eclipse.cdt.core.model.ICElement;
 import org.eclipse.cdt.core.model.ITranslationUnit;
+import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.runtime.CoreException;
 
@@ -33,6 +36,7 @@ public class CxxModelsCache {
 	private ITranslationUnit tu;
 	private IIndex index;
 	private WeakHashMap<IASTFunctionDefinition, IControlFlowGraph> cfgmap = new WeakHashMap<IASTFunctionDefinition, IControlFlowGraph>(0);
+	private ICodanCommentMap commentMap;
 	private static CxxModelsCache instance = new CxxModelsCache();
 
 	public static CxxModelsCache getInstance() {
@@ -77,6 +81,36 @@ public class CxxModelsCache {
 		}
 	}
 
+
+	public ICodanCommentMap getCommentedNodeMap(IASTTranslationUnit ast) {
+		if (this.ast == ast) {
+			try {
+				index.acquireReadLock();
+				try {
+					commentMap = new CodanCommentMap(ASTCommenter.getCommentedNodeMap(ast));
+				} finally {
+					index.releaseReadLock();
+				}
+				return commentMap;
+			} catch (InterruptedException e) {
+				return null;
+			}
+		}
+		throw new IllegalArgumentException("Not cached");
+	}
+
+	public ICodanCommentMap getCommentedNodeMap(IFile file) {
+		try {
+			IASTTranslationUnit ast = getAst(file);
+			return getCommentedNodeMap(ast);
+		} catch (InterruptedException e) {
+			return null;
+		} catch (CoreException e) {
+			Activator.log(e);
+			return null;
+		}
+	}
+
 	/**
 	 * Clear cash for current file
 	 */
@@ -85,6 +119,7 @@ public class CxxModelsCache {
 		ast = null;
 		tu = null;
 		index = null;
+		commentMap = null;
 	}
 
 	public synchronized IIndex getIndex(IFile file) throws CoreException, InterruptedException {

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/ICodanCommentMap.java

@@ -0,0 +1,44 @@
+/*******************************************************************************
+ * Copyright (c) 2009,2010 QNX Software Systems
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    QNX Software Systems (Alena Laskavaia)  - initial API and implementation
+ *******************************************************************************/ 
+
+package org.eclipse.cdt.codan.core.cxx.model;
+
+import java.util.List;
+
+import org.eclipse.cdt.core.dom.ast.IASTComment;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+
+/**
+ * Comment map allows to get comments before of after the specific ast node
+ */
+public interface ICodanCommentMap {
+	/**
+	 * Returns an Collection of comments for the given node (after the node). This list contains all the comments 
+	 * which are assigned to this specific node. If no comments are available an empty
+	 * collection is returned.
+	 * @param node The key to fetch the associated comments.
+	 * @return list of comments
+	 */
+	public List<IASTComment> getTrailingCommentsForNode(IASTNode node);
+	/**
+	 * Returns an Collection of comments for the given node (before the node). This list contains all the comments 
+	 * which are assigned to this specific node. If no comments are available an empty
+	 * collection is returned.
+	 * @param node The key to fetch the associated comments.
+	 * @return list of comments
+	 */
+	public List<IASTComment> getLeadingCommentsForNode(IASTNode node);
+	/**
+	 * @param statement
+	 * @return
+	 */
+	public List<IASTComment> getFreestandingForNode(IASTNode node);
+}