Menu

#1228 UnusedPrivateMethod returns false positives

PMD-5.1.3
closed
None
PMD
3-Major
Bug
5.1.2
2015-01-25
2014-07-21
No

UnusedPrivateMethod returns false positives. It doesn't appear to matter if the method call is nested several times or not

First Example

public ModelAndView viewEntry(Model model, HttpServletRequest request) {
    //call private method.  Flagged Method  
    return view(model, VIEW);
}

/** THIS IS FLAGGED AS UNUSED **/
private ModelAndView view(Model model, String view) {       
    //add values to the model

    //return the correct view
    return new ModelAndView(view, MVC_CONSTANTS.MODEL_KEY, model.asMap());
}

Second Example

public ModelAndView showLineGraph(HttpServletRequest request, HttpServletResponse response) throws ServletException {
    //call private method  USAGE #1.  Method not flagged
    MyObject filter = getGraphInnateFilter(request);

    //LINE GRAPHIC LOGIC 

    //write output to response stream and return
}

public ModelAndView showPieChart(HttpServletRequest request, HttpServletResponse response) throws ServletException {
    //call private method  USAGE #2.  Method not flagg
    MyObject filter = getGraphInnateFilter(request);

    //PIE CHART LOGIC 

    //write output to response stream and return
}

/** This method is NOT flagged as unused **/
private MyObject getGraphInnateFilter(HttpServletRequest request) {
    MyObject filter = new MyObject();

    //call private method.  Flagged method
    setInnateFilterFields(filter, request);
    //perfrom logic

    //return
    return filter;
}

/** THIS IS FLAGGED AS UNUSED **/
private void setInnateFilterFields(MyObject filter, HttpServletRequest request) {
    //add values to filter object
}

Related

Issues: #1156
Issues: #1228
Issues: #1249
Issues: #1251

Discussion

  • Andreas Dangel

    Andreas Dangel - 2014-07-26
    • status: open --> more-info-needed
    • assigned_to: Andreas Dangel
    • Milestone: New Tickets --> PMD-next
     
    • michael beaumont

      Hi Andreas,

      Sorry about my delayed response, I have been out of town. Yes, you have the correct test case setup.

      I have some time today and I'm going to play around with that class and see if I can find anything that triggers this bug specifically.

      Thanks
      MIchael

      From: Andreas Dangel [mailto:adangel@users.sf.net]
      Sent: Saturday, July 26, 2014 4:40 AM
      To: [pmd:bugs]
      Subject: [pmd:bugs] #1228 UnusedPrivateMethod returns false positives

      • status: open --> more-info-needed
      • assigned_to: Andreas Dangel
      • Milestone: New Tickets --> PMD-next
      • Comment:

      Hi,

      I unfortunately can't reproduce your two examples.
      Could you please verify, that I created the correct test case for it?
      These are the test cases I added: https://github.com/pmd/pmd/commit/c730c15d13568f9901492c3f54c4740c9b25b44e

      Thanks,
      Andreas


      [bugs:#1228]http://sourceforge.net/p/pmd/bugs/1228 UnusedPrivateMethod returns false positives

      Status: more-info-needed
      Milestone: PMD-next
      Created: Mon Jul 21, 2014 12:03 PM UTC by michael beaumont
      Last Updated: Thu Jul 24, 2014 01:37 PM UTC
      Owner: Andreas Dangel

      UnusedPrivateMethod returns false positives. It doesn't appear to matter if the method call is nested several times or not

      First Example

      public ModelAndView viewEntry(Model model, HttpServletRequest request) {

      //call private method.  Flagged Method
      
      return view(model, VIEW);
      

      }

      / THIS IS FLAGGED AS UNUSED /

      private ModelAndView view(Model model, String view) {

      //add values to the model
      
      //return the correct view
      
      return new ModelAndView(view, MVC_CONSTANTS.MODEL_KEY, model.asMap());
      

      }

      Second Example

      public ModelAndView showLineGraph(HttpServletRequest request, HttpServletResponse response) throws ServletException {

      //call private method  USAGE #1.  Method not flagged
      
      MyObject filter = getGraphInnateFilter(request);
      
      //LINE GRAPHIC LOGIC
      
      //write output to response stream and return
      

      }

      public ModelAndView showPieChart(HttpServletRequest request, HttpServletResponse response) throws ServletException {

      //call private method  USAGE #2.  Method not flagg
      
      MyObject filter = getGraphInnateFilter(request);
      
      //PIE CHART LOGIC
      
      //write output to response stream and return
      

      }

      / This method is NOT flagged as unused /

      private MyObject getGraphInnateFilter(HttpServletRequest request) {

      MyObject filter = new MyObject();
      
      //call private method.  Flagged method
      
      setInnateFilterFields(filter, request);
      
      //perfrom logic
      
      //return
      
      return filter;
      

      }

      / THIS IS FLAGGED AS UNUSED /

      private void setInnateFilterFields(MyObject filter, HttpServletRequest request) {

      //add values to filter object
      

      }


      Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/pmd/bugs/1228/https://sourceforge.net/p/pmd/bugs/1228

      To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/https://sourceforge.net/auth/subscriptions


      This electronic message transmission and any attachments that accompany it contain information from DRC(r) (Dynamics Research Corporation) or its subsidiaries, or the intended recipient, which is privileged, proprietary, business confidential, or otherwise protected from disclosure and is the exclusive property of DRC and/or the intended recipient. The information in this email is solely intended for the use of the individual or entity that is the intended recipient. If you are not the intended recipient, any use, dissemination, distribution, retention, or copying of this communication, attachments, or substance is prohibited. If you have received this electronic transmission in error, please immediately reply to the author via email that you received the message by mistake and also promptly and permanently delete this message and all copies of this email and any attachments. We thank you for your assistance and apologize for any inconvenience.

       

      Related

      Issues: #1228

  • James Jamieson

    James Jamieson - 2014-07-28

    I have a similar problem, and noticed that as soon as I flip the location of methods in source the issue goes away.

    For example the below source code flags as unused, but as soon as the view method is placed above the viewEntry method the violation goes away.

        /**
     * get the view associated with a GET request
     * @param model the current model
     * @param request the http get request
     * @return associated view
     */
    @RequestMapping(method = RequestMethod.GET, value = URL_MAPPINGS.BULK_ACTION_URL)
    public ModelAndView viewEntry(Model model, HttpServletRequest request,            @RequestParam(value = "fiscalYear", required = true) int fy) {
        return view(model, VIEW, fy);
    }
    
    /**
     * Return a view based off a given model and view
     * @param model the model
     * @param view the view
     * @return a view 
     */
    private ModelAndView view(Model model, String view, int fy) {       
        //add values to the model
        model.addAttribute(FIELDS.FISCAL_YEAR, fy);
    
        //return the correct view
        return new ModelAndView(view, MVC_CONSTANTS.MODEL_KEY, model.asMap());
    }
    
     
  • Andreas Dangel

    Andreas Dangel - 2014-07-28

    Hi James, I could reproduce your problem - thank you for your example. It's not about source location of the methods (at least not anymore) - it's about the @RequestParam annotation of the method parameter. This will be fixed with 5.1.3.
    However, I still can't reproduce the original problem.
    Thanks, Andreas

     
  • aterai

    aterai - 2014-07-31

    I am facing same or similar issue only on PMD 5.1.2:

    C:\temp>C:\pmd-bin-5.1.1\bin\pmd.bat -dir . -rulesets java-unusedcode
    C:\temp>C:\pmd-bin-5.1.2\bin\pmd.bat -dir . -rulesets java-unusedcode
    C:\temp\Foo.java:11:    Avoid unused private methods such as 'makePanel(String,JComponent)'.
    
    import java.awt.*;
    import javax.swing.*;
    
    public class Foo {
      public JComponent makeUI() {
        Box box = Box.createVerticalBox();
        JTextField field = new JTextField();
        box.add(makePanel("aaa", field));
        return box;
      }
      private static JPanel makePanel(String title, JComponent c) {
        JPanel p = new JPanel(new BorderLayout());
        p.setBorder(BorderFactory.createTitledBorder(title));
        p.add(c);
        return p;
      }
      public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
          @Override public void run() {
            createAndShowGUI();
          }
        });
      }
      public static void createAndShowGUI() {
        JFrame f = new JFrame();
        f.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        f.getContentPane().add(new Foo().makeUI());
        f.setSize(320, 240);
        f.setLocationRelativeTo(null);
        f.setVisible(true);
      }
    }
    
     
  • michael beaumont

    I have simplified my test case and found something that appears to be triggering this bug. The difference in my below code is that the method goes from having parameters, to none at all.

    The below code does not flag for UnusedPrivateMethod

    public class Foo extends MultiActionController {
    
        private static final Logger logger = Logger.getLogger(Foo.class);
    
        public DashboardGraphInnateFilter_o getGraphInnateFilter(HttpServletRequest request) {
            setInnateFilterFields();
            return null;
    
        }
    
        private void setInnateFilterFields() { //Not flagged
            logger.info("here");
        }
    }
    

    The below code DOES flag for UnusedPrivateMethod

    public class Foo extends MultiActionController {
    
        private static final Logger logger = Logger.getLogger(Foo.class);
    
        public DashboardGraphInnateFilter_o getGraphInnateFilter(HttpServletRequest request) {
            DashboardGraphInnateFilter_o filter = new DashboardGraphInnateFilter_o();
            setInnateFilterFields(filter, request); 
            return filter;
    
        }
    
        private void setInnateFilterFields(DashboardInnateFilter_o filter, HttpServletRequest request) { //incorrectly flagged
            logger.info("here");
        }
    }
    
     
  • Andreas Dangel

    Andreas Dangel - 2014-08-01

    Alright, the missing piece was the type resolution. I've added this, so that the
    rule can actually detect subclasses. E.g. that DashboardGraphInnateFilter_o is
    a subclass of DashboardInnateFilter_o. The same was the problem with
    JTextField/JComponent.

    You'll need to use the "-auxclasspath" command line option to set the class path,
    so that the type resolution will work.

    Can you please try this with the snapshot version and let me know, whether
    it works?
    https://sourceforge.net/projects/pmd/files/pmd/5.1.3-SNAPSHOT/

     
    • aterai

      aterai - 2014-08-06

      I'm sorry for this late reply.
      I tried to use pmd-bin-5.1.3-SNAPSHOT.zip but I couldn't find lib/pmd-5.1.3.jar in the zip file.

       
      • Andreas Dangel

        Andreas Dangel - 2014-08-06

        Sorry, that's my fault.
        I re-created the zip file, now lib/pmd-5.1.3-SNAPSHOT.jar is contained in the zip.

         
        • aterai

          aterai - 2014-08-07

          Thank you for the quick response.
          I could download the new version, and the UnusedPrivateMethod false positives issues in my project has been substantially modified by the PMD 5.1.3-SNAPSHOT.
          This example will show the remaining issues:

          import java.awt.*;
          import java.awt.event.*;
          import java.util.*;
          import javax.swing.*;
          import javax.swing.tree.*;
          
          public class Bar {
              private final JTree tree = new JTree();
              public JComponent makeUI() {
                  Box box = Box.createVerticalBox();
                  box.add(new JButton(new AbstractAction("expand  ") {
                      @Override public void actionPerformed(ActionEvent e) {
                          TreeNode root = (TreeNode) tree.getModel().getRoot();
                          visitAll(tree, new TreePath(root), true);
                      }
                  }));
                  box.add(new JButton(new AbstractAction("collapse") {
                      @Override public void actionPerformed(ActionEvent e) {
                          TreeNode root = (TreeNode) tree.getModel().getRoot();
                          visitAll(tree, new TreePath(root), false);
                      }
                  }));
                  box.add(Box.createVerticalGlue());
          
                  JPanel p = new JPanel(new BorderLayout());
                  p.add(box, BorderLayout.EAST);
                  p.add(new JScrollPane(tree));
                  return p;
              }
              private static void visitAll(JTree tree, TreePath parent, boolean expand) {
                  TreeNode node = (TreeNode) parent.getLastPathComponent();
                  if (!node.isLeaf() && node.getChildCount() >= 0) {
                      Enumeration e = node.children();
                      while (e.hasMoreElements()) {
                          TreeNode n = (TreeNode) e.nextElement();
                          TreePath path = parent.pathByAddingChild(n);
                          visitAll(tree, path, expand);
                      }
                  }
                  if (expand) {
                      tree.expandPath(parent);
                  } else {
                      tree.collapsePath(parent);
                  }
              }
              public static void main(String[] args) {
                  EventQueue.invokeLater(new Runnable() {
                      @Override public void run() {
                          createAndShowGUI();
                      }
                  });
              }
              public static void createAndShowGUI() {
                  JFrame f = new JFrame();
                  f.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
                  f.getContentPane().add(new Bar().makeUI());
                  f.setSize(320, 240);
                  f.setLocationRelativeTo(null);
                  f.setVisible(true);
              }
          }
          
           
  • Anthony Whitford

    I am running into this too... Is the status still "More Info Needed" because I think the issue is related to sub-classes AND type coercion. In one of my false-positive cases, I have a function that accepts primitive doubles, and I'm passing things like Double, double, and Float.

     
  • Andreas Dangel

    Andreas Dangel - 2014-08-09
    • status: more-info-needed --> closed
     
  • Andreas Dangel

    Andreas Dangel - 2014-08-09

    I'll close this ticket now. I believe, I have fixed all outstanding issues that have been mentioned in this thread, including inheritance, auto-boxing and double/float cases.
    If this is not the case, please open a new ticket with a code sample.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.